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

Handle major versions and non-rc prereleases #43

Merged
merged 4 commits into from
Nov 30, 2023
Merged

Conversation

mtreinish
Copy link
Member

This commit adds support to the qiskit-bot release automation to handle major version releases and non-rc pre-releases. The major version handling is primarily for building the log command, as we need to get the log from 'n.0.0..n-1.m.0' where m is the most recent non-prerelease. To handle non-rc prereleases the only change is we don't create a new branch for the pre-release. Previously all pre-releases would trigger branch creation (given the appropriate configuration variable). But the desired behavior is to actually only create a stable branch from a release candidate.

This commit adds support to the qiskit-bot release automation to handle
major version releases and non-rc pre-releases. The major version
handling is primarily for building the log command, as we need to get
the log from 'n.0.0..n-1.m.0' where m is the most recent non-prerelease.
To handle non-rc prereleases the only change is we don't create a new
branch for the pre-release. Previously all pre-releases would trigger
branch creation (given the appropriate configuration variable). But the
desired behavior is to actually only create a stable branch from a
release candidate.
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

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

This looks sensible to me. The main question is about whether we need to / ought to handle the last-minor-is-only-deprecations version strategy we were thinking of doing for all Qiskit majors here.

As a minor aside - log messages are typically not supposed to be eagerly formatted, right? It's not going to matter here, because there's no real performance concerns, just a minor note.

Comment on lines 232 to 244
# If a minor release log between 0.X.0..0.X-1.0
elif version_obj.major >= 1 and version_obj.minor == 0:
tags = git.get_tags(repo)
previous_major = version_obj.major - 1
for tag in tags.splitlines():
tag_version = parse(tag)
if tag_version.is_prerelease:
continue
if tag_version.major == previous_major:
old_version = tag
break
else:
old_version = f"{version_obj.epoch}.{version_obj.minor - 1}.0"
Copy link
Member

Choose a reason for hiding this comment

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

The comment at the top here looks like it's now attached to the wrong branch of this if/elif chain now.

Also, this might be a bit tricksy with regard to the Qiskit 0.46 strategy - the changelog we'll actually care about will typically be the penultimate minor, in the "final minor is the deprecation boogaloo" style we were planning to do with our majors, I think? With things as currently written, and how we've been merging up from 0.45 to 0.46, I think the ... comparison is going to end up including the bugfixes from 0.45.1+ in the changelog for 1.0? Not certain about that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had forgotten about the 0.46.0 release when I wrote this logic. If we tag 0.46.0 before we tag 1.0.0rc1 then this will be 1.0.0rc1...0.46.0, while if we tag rc1 before 0.46.0 it will be 1.0.0rc1...0.45.x. At the end of the day I don't think it'll make a huge difference as long as we forward port any bugfixes from stable/0.45 to stable/0.46 it will be basically the same history I think.

qiskit_bot/release_process.py Outdated Show resolved Hide resolved
@mtreinish
Copy link
Member Author

As a minor aside - log messages are typically not supposed to be eagerly formatted, right? It's not going to matter here, because there's no real performance concerns, just a minor note.

Yeah that's a good point. The logging in the tool could probably use a bit of an overhaul as it's also a bit higher priority than it should be in a lot of cases. Like the git activity should probably be debug not info

@jakelishman jakelishman merged commit 3441614 into master Nov 30, 2023
5 checks passed
@jakelishman jakelishman deleted the pre-release-not-rc branch November 30, 2023 21:29
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.

2 participants