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

add regex for go1.21 versioning change #359

Merged
merged 1 commit into from
Jul 24, 2023
Merged

add regex for go1.21 versioning change #359

merged 1 commit into from
Jul 24, 2023

Conversation

akhilerm
Copy link
Member

@akhilerm akhilerm commented Jun 28, 2023 •
edited
Loading

go1.21 has changes related to how the go versions are using the version number. Ref: https://tip.golang.org/doc/toolchain#versions

The golang regex need to be updated so that new go versioning format is accepted by publishing bot

@k8s-ci-robot
Copy link
Contributor

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2023
@k8s-ci-robot k8s-ci-robot requested a review from cici37 June 28, 2023 09:26
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Jun 28, 2023
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jun 28, 2023
@akhilerm
Copy link
Member Author

JFYI : @nikhita I think just using the regex to validate will get to complex too understand and maintain. Example impementation.

So I am thinking of a capturing group to get the major, minor , patch and prerelease versions and then validate them.

kubebuilder has something similar here.

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 15, 2023
@akhilerm akhilerm marked this pull request as ready for review July 15, 2023 09:52
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 15, 2023
@akhilerm
Copy link
Member Author

/retest

@akhilerm
Copy link
Member Author

/cc @nikhita
/cc @MadhavJivrajani for the go version checks

@k8s-ci-robot
Copy link
Contributor

@akhilerm: GitHub didn't allow me to request PR reviews from the following users: version, checks, for, the, go.

Note that only kubernetes members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

/cc @nikhita
/cc @MadhavJivrajani for the go version checks

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

MadhavJivrajani
MadhavJivrajani reviewed Jul 17, 2023
View reviewed changes
Copy link

@MadhavJivrajani MadhavJivrajani left a comment

Choose a reason for hiding this comment

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

Leaving a few small comments.

There is a clear distinction between the language version of go and the version of the go toolchain. If I understand correctly, we care about the latter, and that is what we enter into branch rules as well. If my understanding is correct, we should also document GoVersion better in the BranchRule struct.

cmd/publishing-bot/config/rules.go Show resolved Hide resolved
cmd/publishing-bot/config/rules.go Outdated Show resolved Hide resolved
cmd/publishing-bot/config/rules.go Outdated Show resolved Hide resolved
cmd/publishing-bot/config/rules.go Outdated Show resolved Hide resolved
cmd/publishing-bot/config/rules.go Show resolved Hide resolved
cmd/publishing-bot/config/rules.go Outdated Show resolved Hide resolved
MadhavJivrajani
MadhavJivrajani approved these changes Jul 21, 2023
View reviewed changes
Copy link

@MadhavJivrajani MadhavJivrajani left a comment

Choose a reason for hiding this comment

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

/lgtm
I'll defer to @nikhita for the final review and approval.

Thanks @akhilerm!

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

/test pull-publishing-bot-test-kubernetes-master

@nikhita
Copy link
Member

nikhita commented Jul 21, 2023

@akhilerm can you also squash the commmits?

@akhilerm
add regex for go1.21 versioning change
bb380ce 
as part of the go1.21 versioning convention change, go versions <= 1.20
and >=1.21.0 need to be handled separately.

All go versions >= 1.21.0, should have a patch version if it is not a
prerelease.

Signed-off-by: Akhil Mohan <akhilerm@gmail.com>
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 21, 2023
@akhilerm
Copy link
Member Author

/hold
Waiting for kubernetes/test-infra#30151 so that the pull-publishing-bot-test-kubernetes-master can be run without getting timed out.

@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 Jul 21, 2023
@akhilerm
Copy link
Member Author

/unhold
/test pull-publishing-bot-test-kubernetes-master

@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 Jul 23, 2023
nikhita
nikhita approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@nikhita nikhita left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: akhilerm, MadhavJivrajani, nikhita

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 Jul 24, 2023
@k8s-ci-robot k8s-ci-robot merged commit 04b8d2b into kubernetes:master Jul 24, 2023
1 check passed
@akhilerm akhilerm deleted the fix-go-version-regex branch July 24, 2023 10:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MadhavJivrajani MadhavJivrajani MadhavJivrajani approved these changes

@nikhita nikhita nikhita approved these changes

@cici37 cici37 Awaiting requested review from cici37

@jimangel jimangel Awaiting requested review from jimangel

@jrsapi jrsapi Awaiting requested review from jrsapi

@salaxander salaxander Awaiting requested review from salaxander

Assignees

@MadhavJivrajani MadhavJivrajani

@nikhita nikhita

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.
4 participants
@akhilerm @k8s-ci-robot @nikhita @MadhavJivrajani