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

Bug 1786807 - [treescript] Correctly apply esr suffix in version-bump. #1102

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jfx2006
Copy link
Collaborator

@jfx2006 jfx2006 commented Dec 11, 2024

Applies to "release-version-bump" task. The "new" version comes from Shipit during release promotion and will have the esr suffix for "esr" release types.

Applies to "release-version-bump" task. The "new" version comes from
Shipit during release promotion and will have the esr suffix for "esr"
release types.
@jfx2006 jfx2006 marked this pull request as draft December 11, 2024 01:34
@jfx2006
Copy link
Collaborator Author

jfx2006 commented Dec 11, 2024

From the bug description, version_display is the only file that should ever have the "esr" suffix.
My strategy is to check each file to see if it has the esr suffix and if the new version set in the payload has the suffix but the current file does not, drop the suffix from next_version before writing the new file.

I adjusted some tests accordingly. Most tests were writing only to milestone.txt, which by definition should never have an esr (or b_n_) suffix. So I added in the other files in a couple of places. The "test_bump_version" test with its "should_append_esr" parameter is removed since bump_version will always have the new version passed as an argument.

One could argue that bump_version is a poorly named function as it really sets the new version it was given.

In the interest of minimal change, I did not reuse the merge automation logic as suggested because merge automation works differently - given a file and a version field, it calculates the new version and then calls do_bump_version whereas version-bump is told what version to use in the payload.

@jfx2006 jfx2006 marked this pull request as ready for review December 11, 2024 01:49
@jfx2006 jfx2006 marked this pull request as draft December 11, 2024 01:51
@jfx2006
Copy link
Collaborator Author

jfx2006 commented Dec 11, 2024

It occurred to me that this may break the release2esr merge automation. I will double check that.

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.

1 participant