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.

Sign up for GitHub

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

Jump to bottom

fix #186706: export 7sus chord to MusicXML fails reimport #3124

Merged
merged 1 commit into from
Apr 11, 2017
Merged

fix #186706: export 7sus chord to MusicXML fails reimport #3124

merged 1 commit into from
Apr 11, 2017

Conversation

Jojo-Schmitz
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz commented Apr 10, 2017 •
edited
Loading

simpler alternative to #3123
Should go into 2.1 too

@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Apr 10, 2017
Merged
@MarcSabatella
Copy link
Contributor

In principle I like this simpler version better. But it will need testing to be sure there are no further regressions.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Apr 10, 2017 •
edited
Loading

It is adding that text="" in exactly the same cases as before, just at 2 different (and as per Michael Good correct) places. The old place was wrong but apparently nobody stumbled accross it since it got added in 2.0.2, until 2.1-RC, so for 2 years. So worst that can happen is that this is still wrong, but it should not be wronger than before. MuseScore imports it correctly, so it does pass its XML validator.

@lasconic
Copy link
Contributor

needs rebase.

@lasconic
Copy link
Contributor

wait... I just saw the comment from the previous one... what a mess...

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Apr 11, 2017 •
edited
Loading

Hmm, it was an alternative to #3123, that is merged (and causes the conflict), so this here won't be needed anymore? It was prefered though by at least 3 reviewers. I guess reverting #3123 and then merging this here should do?

@lasconic lasconic merged commit 282d905 into musescore:master Apr 11, 2017
@lasconic
Copy link
Contributor

yes I saw that but too late.

@lasconic
Copy link
Contributor

Done.

@Jojo-Schmitz
Copy link
Contributor Author

Sorry, I though the comments to have been clear enough ;-)

@Jojo-Schmitz Jojo-Schmitz deleted the exportxml2 branch April 11, 2017 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Jojo-Schmitz @MarcSabatella @lasconic