-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[release tool] Integrate release and hashrelease logic #9140
[release tool] Integrate release and hashrelease logic #9140
Conversation
0c68b6a
to
8867f3d
Compare
383fc3f
to
a97a86d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not think this should change the output of hashrelease currently since that will affect testing as is
cbf9d3f
to
b7dfe38
Compare
release/internal/version/version.go
Outdated
// is the _next_ (currently unreleased) product version. | ||
func IsDevVersion(ver, devTag string) bool { | ||
v := Version(ver) | ||
pattern := fmt.Sprintf(`^v\d+\.\d+\.\d+(-\d+\.\d+)?-%s-\d+-g[0-9a-f]{12}(-dirty)?$`, devTag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should allow interpreting v3.15.0-12-gfd40f1838223
as a dev tag as well
pattern := fmt.Sprintf(`^v\d+\.\d+\.\d+(-\d+\.\d+)?-%s-\d+-g[0-9a-f]{12}(-dirty)?$`, devTag) | |
pattern := fmt.Sprintf(`^v\d+\.\d+\.\d+(-\d+\.\d+)?(-%s)?-\d+-g[0-9a-f]{12}(-dirty)?$`, devTag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why should this return true if the version string doesn't include the devTag
? I thought the entire purpose of this function was to tell if the dev tag was present.
That said, this function is currently unused so I think it can be removed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it can be removed
This function was going based on a wrong assumption that all dev tags (non-official release) has the devTag 0.dev
.
That is only the case for minor version releases (i.e. vX.Y.0-0.dev-n-g????????????
); patch releases go based on the last release (i.e. vX.Y.Z-n-g????????????
)
2083711
to
1d85444
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
otherwise lgtm
release/internal/version/version.go
Outdated
// is the _next_ (currently unreleased) product version. | ||
func IsDevVersion(ver, devTag string) bool { | ||
v := Version(ver) | ||
pattern := fmt.Sprintf(`^v\d+\.\d+\.\d+(-\d+\.\d+)?-%s-\d+-g[0-9a-f]{12}(-dirty)?$`, devTag) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, it can be removed
This function was going based on a wrong assumption that all dev tags (non-official release) has the devTag 0.dev
.
That is only the case for minor version releases (i.e. vX.Y.0-0.dev-n-g????????????
); patch releases go based on the last release (i.e. vX.Y.Z-n-g????????????
)
Description
Note that this DOES change the format of our hashreleases in order to better match the structure of artifacts produced by a real release. Notably, existing hashrelease structure looks like this:
The new format looks more like this:
The main differences being:
To handle this, there is some additional logic to copy files into the "legacy" structure so that both formats coexist. Once our CI tools are updated to use the new format, we can remove this redundancy.
Related issues/PRs
Todos
Release Note
Reminder for the reviewer
Make sure that this PR has the correct labels and milestone set.
Every PR needs one
docs-*
label.docs-pr-required
: This change requires a change to the documentation that has not been completed yet.docs-completed
: This change has all necessary documentation completed.docs-not-required
: This change has no user-facing impact and requires no docs.Every PR needs one
release-note-*
label.release-note-required
: This PR has user-facing changes. Most PRs should have this label.release-note-not-required
: This PR has no user-facing changes.Other optional labels:
cherry-pick-candidate
: This PR should be cherry-picked to an earlier release. For bug fixes only.needs-operator-pr
: This PR is related to install and requires a corresponding change to the operator.