-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
DlgPrefRecord: enable CUE sheet recording by default #3374
Conversation
ffac0aa
to
58d2cd9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have the default value now scattered among the code base.
Please use a single constant.
A CUE sheet cannot be created retroactively after making a recording, so it is better to have it on by default. If a user does not want them, they can delete the CUE files and turn off the option easily. As discussed on Zulip https://mixxx.zulipchat.com/#narrow/stream/109122-general/topic/Exporting.20a.20Mixcloud-digestable.20playlist
58d2cd9
to
c9e6e19
Compare
Okay, done. I will not do anymore refactoring of this legacy code. I am only changing this one boolean value; that's it. This preferences code needs an overhaul so that it is not possible to have the default value inconsistent in different places. |
Unrelated: I see varying and spurious(?) test failures on different Windows CI builds. This is strange. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Thank you.
A CUE sheet cannot be created retroactively after making a
recording, so it is better to have it on by default. If a user
does not want them, they can delete the CUE files and turn off
the option easily.
As discussed on Zulip
https://mixxx.zulipchat.com/#narrow/stream/109122-general/topic/Exporting.20a.20Mixcloud-digestable.20playlist