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.

Sign up for GitHub

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

Jump to bottom

🌱 Bump internal versions to use release v0.4.0 #56

Merged
merged 1 commit into from
Oct 25, 2023
Merged

🌱 Bump internal versions to use release v0.4.0 #56

merged 1 commit into from
Oct 25, 2023

Conversation

killianmuldoon
Copy link
Contributor

Update the internally referenced version of the action and the notes tool to the current release.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Oct 23, 2023
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Oct 23, 2023
killianmuldoon
killianmuldoon commented Oct 23, 2023
View reviewed changes
Copy link
Contributor Author

@killianmuldoon killianmuldoon left a comment

Choose a reason for hiding this comment

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

/hold

This is breaking the changes made for Cluster API in #55 as the version used in the hosted action does not include those changes.

AFAICT the version here should probably be bumped to v0.4.1 - is my understanding correct?

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 23, 2023
g-gaston
g-gaston reviewed Oct 23, 2023
View reviewed changes
Copy link

@g-gaston g-gaston left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 23, 2023
@sbueringer
Copy link
Member

I would guess it should be v0.4.1 to match the upcoming release.

@vincepri @camilamacedo86?

killianmuldoon
killianmuldoon commented Oct 23, 2023
View reviewed changes
verify/go.mod
@@ -7,7 +7,7 @@ replace sigs.k8s.io/kubebuilder-release-tools/notes => ../notes
require (
github.com/google/go-github/v32 v32.1.0
golang.org/x/oauth2 v0.8.0
sigs.k8s.io/kubebuilder-release-tools/notes v0.3.0
sigs.k8s.io/kubebuilder-release-tools/notes v0.4.0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This version doesn't specifically matter because of the replace, but probably better to keep it to latest in any case.

Copy link
Member

Choose a reason for hiding this comment

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

we can get this one merged and then do another one
or just set here the latest version.

camilamacedo86
camilamacedo86 approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, killianmuldoon

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 24, 2023
@killianmuldoon
Copy link
Contributor Author

killianmuldoon commented Oct 24, 2023 •
edited
Loading

@camilamacedo86 are you likely to do a new release of 0.4.X for this? In that case I'll bump the version in this PR to v0.4.1

@sbueringer
Copy link
Member

sbueringer commented Oct 24, 2023 •
edited
Loading

@camilamacedo86 are you likely to do a new release of 0.4.X for this? In that case I'll bump the version in this PR to v0.4.1

@killianmuldoon Is our action taking the version from the master branch or from the tag? If it's from the tag I would prefer v0.4.1 & a new patch release to have it clean

@killianmuldoon
Copy link
Contributor Author

@killianmuldoon Is our action taking the version from the master branch or from the tag? If it's from the tag I would prefer v0.4.1 & a new patch release to have it clean

It's from the tag - but my question is to figure out the preference of whoever is doing the release. I'll update the PR regardless

@killianmuldoon
Bump release-verifier action to use release v0.4.0
b69a151 
Signed-off-by: killianmuldoon <kmuldoon@vmware.com>
@killianmuldoon killianmuldoon force-pushed the pr-update-version-v0.4.0 branch from 74c9832 to b69a151 Compare October 24, 2023 16:56
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 24, 2023
@sbueringer
Copy link
Member

sbueringer commented Oct 25, 2023 •
edited
Loading

I think it would be pretty bad if we basically consume a tag which points to another version (e.g. we pin to v0.4.1 and that links to v0.4.0). But I know that's not what you're suggesting. I was just trying to understand how it works :)

But of course for this PR here it depends on if we just bring main in line with the last release (so set it to v0.4.0) or if we already set it up for a v0.4.1 release. I would prefer the latter, but as you said of course up to maintainers.

@sbueringer
Copy link
Member

sbueringer commented Oct 25, 2023 •
edited
Loading

/lgtm

from my side, but should be reviewed again by a maintainer before merge

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 25, 2023
camilamacedo86
camilamacedo86 reviewed Oct 25, 2023
View reviewed changes
Copy link
Member

@camilamacedo86 camilamacedo86 left a comment

Choose a reason for hiding this comment

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

@vincepri is who has been taken care of this one.
I am OK with the changes, but let's see what he thinks.

@vincepri
Copy link
Member

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Oct 25, 2023
@k8s-ci-robot k8s-ci-robot merged commit e389278 into kubernetes-sigs:master Oct 25, 2023
3 checks passed
@sbueringer
Copy link
Member

@vincepri / @camilamacedo86 Can one of you please create a new v0.4.1 release?

We currently use v0.4.0 of the action (which uses the v0.3.0 image) in Cluster API and we would like to resolve this situation

@mjlshen mjlshen mentioned this pull request Nov 1, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@camilamacedo86 camilamacedo86 camilamacedo86 approved these changes

@g-gaston g-gaston g-gaston left review comments

@droot droot Awaiting requested review from droot

Assignees

@sbueringer sbueringer

@g-gaston g-gaston

@camilamacedo86 camilamacedo86

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@killianmuldoon @sbueringer @k8s-ci-robot @vincepri @g-gaston @camilamacedo86