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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃尡 Increment internal versions to use release v0.4.3 #58

Merged
merged 1 commit into from Nov 29, 2023

Conversation

mjlshen
Copy link
Contributor

@mjlshen mjlshen commented Nov 1, 2023

This will allows users of this GitHub action to use the changes from #56

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 1, 2023
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 1, 2023
@mjlshen mjlshen force-pushed the 0.4.3 branch 3 times, most recently from e532683 to 1138402 Compare November 1, 2023 16:43
Copy link
Contributor

@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.

/lgtm

Thanks!

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 1, 2023
@mjlshen
Copy link
Contributor Author

mjlshen commented Nov 1, 2023

cc @vincepri for approval

Copy link
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

/lgtm

@@ -9,4 +9,4 @@ runs:
# this is built using GCB by building the Dockerfile in this directory on
# every tagged release. After tagging a release, a new version should
# automatically appear.
image: 'docker://gcr.io/kubebuilder/pr-verifier:v0.4.1'
Copy link
Member

@sbueringer sbueringer Nov 4, 2023

Choose a reason for hiding this comment

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

I think this should be v0.4.3 now.

We wouldn't want a kube-builder-tools v0.4.3 release that points to a v0.4.2 image?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy to make the change to v0.4.3 and have done so since it's easy enough to revert - adding @killianmuldoon 's point below this PR should probably only be merged if a 0.4.3 release if cut right after, otherwise the v0.4.3 image won't actually exist.

@killianmuldoon
Copy link
Contributor

I think this should be v0.4.3 now.

Only if there's certain to be a release with that version - i.e. this would be a release PR. If we want to use the sha of a commit using the current version with an image that exists is the right approach IMO

Signed-off-by: Michael Shen <mishen@umich.edu>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 5, 2023
@sbueringer
Copy link
Member

Ah I see

@sbueringer
Copy link
Member

/lgtm

@vincepri @camilamacedo86 Can we merge this PR and cut a new v0.4.3 release? So we have a release without the panic and a release which uses the same image (v0.4.3) as the release tag (v0.4.3)

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 13, 2023
@mjlshen mjlshen changed the title 馃尡 Increment internal versions to use release v0.4.2 馃尡 Increment internal versions to use release v0.4.3 Nov 14, 2023
@@ -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.4.0
sigs.k8s.io/kubebuilder-release-tools/notes v0.4.2
Copy link
Member

Choose a reason for hiding this comment

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

while we can't bump this to non-existent version as we are doing in other file with image version, would not it be problem (not having our fix included in the tool) when using it from CAPI?

Copy link
Member

Choose a reason for hiding this comment

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

This version is not used at all, see the replace in l.5.

Maybe we should just set it to v0.0.0 (if possible) to avoid confusion

Copy link
Member

Choose a reason for hiding this comment

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

@sbueringer I see, have not expanded the file fully to see the replace.. Then it should be all good 馃憤馃徏

Copy link
Member

Choose a reason for hiding this comment

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

Yup. It's just pretty confusing as this version looks like something that almost makes sense, but can never actually be the same version

Copy link
Member

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

cc @vincepri PTAL

We (CAPI release team) would appreciate a new release once this is merged, thanks!

Copy link
Member

@vincepri vincepri left a comment

Choose a reason for hiding this comment

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

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjlshen, vincepri

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 Nov 29, 2023
@k8s-ci-robot k8s-ci-robot merged commit 012269a into kubernetes-sigs:master Nov 29, 2023
6 checks passed
@mjlshen mjlshen deleted the 0.4.3 branch November 29, 2023 21:49
@mjlshen
Copy link
Contributor Author

mjlshen commented Nov 29, 2023

@vincepri Thanks for getting this merged in! Could you help get a v0.4.3 release cut as well? Unfortunately the code that got merged won't work without it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants