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

[algorithms] Change stable tag 'mismatch' to alg.mismatch, so that it is consistent with similar labels in the same section #6653

Merged
merged 1 commit into from Nov 10, 2023

Conversation

ojrosten
Copy link
Contributor

No description provided.

Copy link
Contributor

@JohelEGP JohelEGP left a comment

Choose a reason for hiding this comment

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

This needs an entry into xrefdelta.tex.

@ojrosten
Copy link
Contributor Author

Thanks! And just to be clear it would look something like this?

% #6653
\movedxref{algorithms.alg.nonmodifying.mismatch}{algorithms.alg.nonmodifying.alg.mismatch}

@JohelEGP
Copy link
Contributor

It's this example: \movedxref{old.label}{new.label}.
The old label is mismatch, and the new label alg.mismatch.

@JohelEGP
Copy link
Contributor

@ojrosten
Copy link
Contributor Author

Noted; I'm very much a rookie ;)

@ojrosten ojrosten changed the title Correct stable tag for mismatch to alg.mismatch [mismatch] Change stable tag 'mismatch' to alg.mismatch, so that it is consistent with similar labels in the same section Nov 10, 2023
@ojrosten
Copy link
Contributor Author

Hopefully the title of the PR is now fixed...

@JohelEGP
Copy link
Contributor

It should cover the commit message, not just the issue's title.
Also, the label should probably be [algorithms],
since you're changing both the [algorithm.syn] and [mismatch] within.

@ojrosten
Copy link
Contributor Author

The label in the title?

@JohelEGP
Copy link
Contributor

In the commit message, according to the first example at https://github.com/cplusplus/draft/wiki/Commit-message-format#editorial-commits.

@ojrosten ojrosten changed the title [mismatch] Change stable tag 'mismatch' to alg.mismatch, so that it is consistent with similar labels in the same section [algorithms] Change stable tag 'mismatch' to alg.mismatch, so that it is consistent with similar labels in the same section Nov 10, 2023
@ojrosten
Copy link
Contributor Author

How important is it to change the commit messages? I can of course do so, but I gather that github strongly recommends against force pushing and I'm not aware of any other way.

@JohelEGP
Copy link
Contributor

PRs are generally a working space that are OK to force-push to.

@ojrosten
Copy link
Contributor Author

I'll do this then :)

I'll prepend the first commit message with [algorithms]. But what about the second, i.e. the change to xrefdelta.tex?

@jensmaurer
Copy link
Member

I'll prepend the first commit message with [algorithms]. But what about the second, i.e. the change to xrefdelta.tex?

xrefdelta is just technical fall-out; you should ignore this for the commit message.

@JohelEGP
Copy link
Contributor

It's all part of the bigger change, so you can rebase to fixup.
That is, git rebase -i main, and change the second commit from pick to fixup.

@ojrosten
Copy link
Contributor Author

Hopefully that's now done. Thank you so much for your help! (And of course I'm happy to make further changes if something isn't right.)

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

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

While we generally frown upon changing stable labels, this seems to fix a rather disturbing inconsistency that outweighs other concerns. (Also, we expect few references to that section to exist.)

@tkoeppe tkoeppe merged commit f0c172c into cplusplus:main Nov 10, 2023
2 checks passed
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