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

[MU4] Fix #334025: Export missing instrument-sound tag #12680

Merged
merged 2 commits into from
May 16, 2023
Merged

[MU4] Fix #334025: Export missing instrument-sound tag #12680

merged 2 commits into from
May 16, 2023

Conversation

Jojo-Schmitz
Copy link
Contributor

Resolves: https://musescore.org/en/node/334025

Submitting this PR on behalf of @luto65

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 4, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
4630bd7 
Backport of musescore#12680
@Jojo-Schmitz Jojo-Schmitz force-pushed the musicxml_instrument-sound branch 2 times, most recently from 16025a6 to e8268b7 Compare August 4, 2022 12:22
@RobFog
Copy link

RobFog commented Aug 4, 2022

Is this something for @lvinken to look at?

@Jojo-Schmitz
Copy link
Contributor Author

It is based on his proposal at least ;-)

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 4, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
cd12f63 
Backport of musescore#12680
@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Aug 4, 2022
Closed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 4, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
4953bd6 
Backport of musescore#12680
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 4, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
016ed7b 
Backport of musescore#12680
@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Aug 4, 2022 •
edited
Loading

Anyone knows an easy way to (batch-)fix all these (Music)XML fiiles that otherwise fail the unit tests?
I'm way too lazy to manually fix all these 178 failing tests

@cbjeukendrup
Copy link
Contributor

There used to be an updateReferences script somewhere in the repo... but I can't find it, and if we would find it, it's probably dramatically outdated so won't be useful.

If you can get the tests to run locally, you can try to find out where the files go that are produced by the tests, and copy them from there to the ref files in the repo. But I don't know from memory where to look.

@Jojo-Schmitz
Copy link
Contributor Author

That updateReferences is still there, but just copies files, not regenerates them

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Aug 4, 2022 •
edited
Loading

OK, for now I just circumvent the problem and disable that new functionality in test mode ;-)

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 4, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
5eadfb0 
Backport of musescore#12680

Todo: fix the XML mtest files rather than preventing the tests to fail.
@lvinken
Copy link
Contributor

lvinken commented Aug 5, 2022

Comments from my side: I am aware of this change and agree with it. I would recommend against simply updating all testfiles. Almost all of them do not contain an instrument-sound element, which means they is generated on import and that needs to be checked for correctness. In 3.x there was indeed an updateReference script in mtest/musicxml/io, but I am not aware of its origin.

@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Aug 5, 2022 •
edited
Loading

OK, so I'll leave those disabled in test mode. The failing tests can get checked on in the 1st commit, it is all about that one line per instrument with instrument-sound missing, in 178 tests.

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Aug 5, 2022
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
232b188 
Backport of musescore#12680
@Jojo-Schmitz
Copy link
Contributor Author

Jojo-Schmitz commented Jan 28, 2023 •
edited
Loading

Can this PR please get reviewed and merged?

@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Jan 30, 2023
Closed
@Jojo-Schmitz
Copy link
Contributor Author

@RomanPudashkin mind to review and merge?

@Jojo-Schmitz Jojo-Schmitz mentioned this pull request Mar 1, 2023
Closed
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 5, 2023
@Jojo-Schmitz
Fix #334025: Export missing instrument-sound tag
3b40f5d 
Backport of musescore#12680
@Jojo-Schmitz Jojo-Schmitz force-pushed the musicxml_instrument-sound branch 3 times, most recently from 66d0f8a to 82f3ad2 Compare March 18, 2023 12:51
@Jojo-Schmitz
Copy link
Contributor Author

@RomanPudashkin mind to review and merge?

@Jojo-Schmitz
Copy link
Contributor Author

@RomanPudashkin mind to review and merge?

@RomanPudashkin RomanPudashkin merged commit 29e96ff into musescore:master May 16, 2023
@Jojo-Schmitz Jojo-Schmitz deleted the musicxml_instrument-sound branch May 16, 2023 09:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbjeukendrup cbjeukendrup cbjeukendrup approved these changes

@RomanPudashkin RomanPudashkin Awaiting requested review from RomanPudashkin

Assignees
No one assigned
Labels
MusicXML
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Jojo-Schmitz @RobFog @cbjeukendrup @lvinken @oktophonie @RomanPudashkin