-
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
📖 Updated version pattern matching in quick-start #6137
Conversation
|
Welcome @joekr! |
Hi @joekr. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
@joekr Thank you very much! /lgtm |
docs/book/src/user/quick-start.md
Outdated
@@ -101,7 +101,10 @@ The clusterctl CLI tool handles the lifecycle of a Cluster API management cluste | |||
#### Install clusterctl binary with curl on linux | |||
Download the latest release; on linux, type: | |||
``` | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.0.x"}} -o clusterctl | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.x" |
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.
@joekr I don't think we should drop the minor version, given that the code will be moved in a branch and the book version generated on top of it should match the same branch.
Also, somehow related, before fixing this we should figure out #6017
@sbueringer are there updates from netlify?
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.
Also, somehow related, before fixing this we should figure out #6017
@sbueringer are there updates from netlify?
I didn't get a reply-mail, maybe @CecileRobertMichon
Good point, but think it should be fine to change it to 1.1.x already though to get the current book fixed. I would prefer if we have a 1.1 version there now (and cherry-pick to release-1.1). The release-1.0 branch will then still produce a book with links to 1.0.x
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.
Updated
082c2c6
to
a2b6e66
Compare
a2b6e66
to
a09dbf5
Compare
docs/book/src/user/quick-start.md
Outdated
@@ -101,7 +101,10 @@ The clusterctl CLI tool handles the lifecycle of a Cluster API management cluste | |||
#### Install clusterctl binary with curl on linux | |||
Download the latest release; on linux, type: | |||
``` | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.0.x"}} -o clusterctl | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.1.x" | |||
prereleases:"true" |
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 need to remove this. was for testing 😮💨
a09dbf5
to
01b597c
Compare
docs/book/src/user/quick-start.md
Outdated
@@ -101,7 +101,7 @@ The clusterctl CLI tool handles the lifecycle of a Cluster API management cluste | |||
#### Install clusterctl binary with curl on linux | |||
Download the latest release; on linux, type: | |||
``` | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.0.x"}} -o clusterctl | |||
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.1.x" }} -o clusterctl |
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.
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.1.x" }} -o clusterctl | |
curl -L {{#releaselink gomodule:"sigs.k8s.io/cluster-api" asset:"clusterctl-linux-amd64" version:"1.1.x"}} -o clusterctl |
last nit
01b597c
to
8f82de9
Compare
/lgtm I think independent of how we keep/change our book versioning this is a good fix for main (and also to cherry-pick into release-1.1) |
@joekr Can you please look into signing the CLA via EasyCLA? (#6137 (comment)) |
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.
/lgtm
@joekr congrats on your first PR in CAPI 🥳 ! /cherry-pick release-1.1 |
@fabriziopandini: once the present PR merges, I will cherry-pick it on top of release-1.1 in a new PR and assign it to you. In response to this:
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. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini 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 |
@sbueringer should this go in our release task? |
@fabriziopandini: new pull request created: #6148 In response to this:
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. |
Good point. Done |
What this PR does / why we need it:
The old
1.0.x
wasn't matching the new version. Instead of making thechange now for each new minor release this will match all minor and
patch releases.
Related issue: #6136