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

Develop metersig #1020

Merged
merged 8 commits into from
Oct 11, 2022
Merged

Conversation

lpugin
Copy link
Member

@lpugin lpugin commented Oct 3, 2022

Add sym+norm to data.METERFORM

Add other as possible value in att.meterSigGrp.log@func

Closes #504

* First step in implementing music-encoding#504 (comment)
* At this stage, att.meterSig.log and att.meterSigDefault.log remain empty. The are still referenced in the schema
* Second step for the implementation of music-encoding#504 (comment)
* A small difference with the proposal is the value, namely "sym+norm" rather than "norm+sym" because the common practice is to show the symbol before the numbers.
* This step of the proposal music-encoding#504 (comment)
* Instead of "unknown", the attribute value added is "other" because it seemed more appropriate
@github-actions github-actions bot added the Component: Core Schema changes to source/modules/* (assigned automatically) label Oct 3, 2022
@lpugin
Copy link
Member Author

lpugin commented Oct 8, 2022

Anything problematic with this PR? Pinging @kepper and @musicEnfanthen

source/modules/MEI.xml Outdated Show resolved Hide resolved
source/modules/MEI.cmn.xml Outdated Show resolved Hide resolved
@musicEnfanthen
Copy link
Member

@lpugin No, just two small questions about clarification. Thanks for the reminder

@musicEnfanthen
Copy link
Member

Also, would we need to adjust the desc of meterSigGrp element? https://music-encoding.org/guidelines/dev/elements/meterSigGrp.html

– Used to capture alternating, interchanging, mixed or any other non-standard meter signatures. [??]

lpugin and others added 2 commits October 9, 2022 00:13
Adjust description for sym+norm

Co-authored-by: Stefan Münnich <stefan.muennich@unibas.ch>
Copy link
Contributor

@pe-ro pe-ro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A modest change to the wording of the description for clarity:

Used to capture alternating, interchanging, mixed or other non-standard meter signatures.

pe-ro
pe-ro previously approved these changes Oct 9, 2022
Copy link
Contributor

@pe-ro pe-ro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks.

@musicEnfanthen
Copy link
Member

@pe-ro May I ask you to re-review since Github dismisses a former review when new commits are added to a PR. Many thanks!

@musicEnfanthen musicEnfanthen merged commit c314cf0 into music-encoding:develop Oct 11, 2022
@musicEnfanthen
Copy link
Member

Thank you

@lpugin lpugin deleted the develop-metersig branch October 11, 2022 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Schema changes to source/modules/* (assigned automatically)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarify meterSig sym / form and numbers
3 participants