Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored inspectTime into previewTime #1885

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Choromanski
Copy link
Contributor

  • I agree to license my contribution under LGPL-3.0 or my contribution is from another project with a license compatible with LGPL-3.0

To test this pull request, follow the instructions in the wiki.


Looking through the source code I noticed that these two functions are extremely similar and thought there was two options to clean it up:

  1. Merge the two functions into one [Implemented this one]
  2. Simplify the old previewTime function by removing the skipToEndTime argument and implementing functionality for it in the inspectTime function

@ajayyy
Copy link
Owner

ajayyy commented Oct 19, 2023

I appretiate the try but I think this makes the code less understandble

@Choromanski
Copy link
Contributor Author

Yeah it kills the readability, if you are looking for more readable code would something like this be preferred as Inspect and End buttons are functionally identical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants