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

Fix #36451 : Alla breve forced to 2/2 #1375

Merged
merged 2 commits into from Oct 19, 2014
Merged

Fix #36451 : Alla breve forced to 2/2 #1375

merged 2 commits into from Oct 19, 2014

Conversation

mgavioli
Copy link
Contributor

Fix #36451 : Alla breve forced to 2/2

Basically reverts commit 7ab8acd which was intended to fix issue http://musescore.org/en/node/14548 (apparently no longer needed)

Fix code is kept but commented out, with detailed comment and a small addition, in case the original issue pops up again in the future.

See http://musescore.org/en/node/36541 for more details, score samples and discussion.

Basically reverts commit 7ab8acd which was intended to fix issue http://musescore.org/en/node/14548 (apparently no longer needed)

Fix code is kept but commented out, with detailed comment and a small addition, in case the originanl issue pops up again in the future.

See http://musescore.org/en/node/36541 for more details, score samples and discussion.
@mgavioli
Copy link
Contributor Author

Hmmm... the fact is that I distinctively dislike code which alters what the user had set. In your specific case, you didn't set anything, it was a previous bug. But a time sig. with 4/4 values and alla breve symbol is possible via the time sig. props dlg box and that code would wipe it out.

I do not have a use case for it right on the spot, but I didn't have use cases for many features other people requested and many peoples did not know (or think) about time sigs requirements like the score I posted in the issue tracker.

So, I am worried about such a kind of code, in itself and as a possibly dangerous precedent.

@Jojo-Schmitz
Copy link
Contributor

Ah, I see, and agree :-)

@lasconic
Copy link
Contributor

could we just delete the code and put this comment in the issue tracker for the related issues? it feels a bit weird to have all this in the code, and I'm not a big fan of seeing this kind of things a little bit everywhere. Also, one day, this comment will just be deleted and lost (except for git history), while in the issue tracker it would remain "forever".

@mgavioli
Copy link
Contributor Author

If this is the policy of the 'company', ok. Comment removed.

However, it is true that in the tracker the info will remain "forever", but it is unlikely that someone thinking about adding back a similar code will look for that info in old, potentially not clearly related, bug discussions.

@lasconic
Copy link
Contributor

Not a policy, just my personal opinion.

@lasconic
Copy link
Contributor

The bug in question is #36541 not #36451

lasconic added a commit that referenced this pull request Oct 19, 2014
…_2_2

Fix #36451 : Alla breve forced to 2/2
@lasconic lasconic merged commit ee0798f into musescore:master Oct 19, 2014
@mgavioli mgavioli deleted the Fix_36451_Alla_breve_forced_to_2_2 branch October 20, 2014 11:18
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

4 participants