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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📖 Update go Prerequisites for the latest release #2257

Merged

Conversation

swiftslee
Copy link
Contributor

Signed-off-by: yuswift yuswift2018@gmail.com
Fixes #2207
As #2182 upgraded the go version to v1.16, the quickstart prerequisites need to be updated too.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 7, 2021
@k8s-ci-robot
Copy link
Contributor

Hi @yuswift. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

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.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 7, 2021
docs/book/src/quick-start.md Outdated Show resolved Hide resolved
@@ -9,7 +9,7 @@ This Quick Start guide will cover:

## Prerequisites

- [go](https://golang.org/dl/) version v1.15+ and < 1.16.
- [go](https://golang.org/dl/) version 1.15+ and < 1.17.
Copy link
Member

@camilamacedo86 camilamacedo86 Jul 11, 2021

Choose a reason for hiding this comment

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

We need to update it for 1.16+ < 1.17. IHMO
c/c @rashmigottipati could you look in the issue which is with you about this topic?
c/c @estroz @jmrodri

Copy link
Contributor Author

Choose a reason for hiding this comment

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

IMO, using go 1.15 is more suitable for kubebuilder v3.0. What about changing it to

- [go](https://golang.org/dl/) version v1.15+ and < 1.16(kubebuilder v3.0).   
- [go](https://golang.org/dl/) version v1.16 and < 1.17(kubebuilder v3.1).

Copy link
Member

Choose a reason for hiding this comment

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

The supported version is the version that is scaffolded. See: https://github.com/kubernetes-sigs/kubebuilder/blob/v3.0.0/testdata/project-v3/go.mod#L3
I agree with your suggestion. It seems fine.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Signed-off-by: yuswift <yuswift2018@gmail.com>
@swiftslee swiftslee force-pushed the fix_quickstart_prerequisites branch from 281e422 to 8306b8f Compare July 16, 2021 11:55
@swiftslee
Copy link
Contributor Author

@camilamacedo86 hi could you add a comment /test all ? Thanks.

@jmcshane
Copy link

hey team, just wanted to put out a note about how important this PR is for the docs. I understand that the go version is reflected in the code, but to keep this out of date with the latest kubebuilder (especially when the book suggests you install the latest version https://book.kubebuilder.io/quick-start.html#installation) can cause a bit of an issue getting up and running with the project. appreciate your work here to keep things up to date.

@estroz
Copy link
Contributor

estroz commented Aug 10, 2021

I've refactored the go version check so versions are compared to plugin-specific requirements (#2290). There is no max version (less that 2.0 which will have breaking changes) since all 1.Y go versions will be forward-compatible, so you can remove the max one it merges.

@swiftslee
Copy link
Contributor Author

I've refactored the go version check so versions are compared to plugin-specific requirements (#2290). There is no max version (less that 2.0 which will have breaking changes) since all 1.Y go versions will be forward-compatible, so you can remove the max one it merges.

Thanks. So, do I just need to keep the min one(e.g. v3.0->go 1.15+ v3.1-> go1.16+)?

@estroz
Copy link
Contributor

estroz commented Aug 11, 2021

Yep!

@swiftslee
Copy link
Contributor Author

Yep!

awesome!

Signed-off-by: yuswift <yuswift2018@gmail.com>
@swiftslee
Copy link
Contributor Author

@estroz hi, I updated the go version. Could you take a look at it? Thanks!

@estroz
Copy link
Contributor

estroz commented Aug 12, 2021

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 12, 2021
Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>
@estroz
Copy link
Contributor

estroz commented Aug 13, 2021

/retest

@estroz
Copy link
Contributor

estroz commented Aug 13, 2021

/approve
/lgtm

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

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: estroz, yuswift

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 Aug 13, 2021
@swiftslee
Copy link
Contributor Author

@estroz Could we merge this pr?

@estroz
Copy link
Contributor

estroz commented Aug 18, 2021

/override coverage/coveralls

@k8s-ci-robot
Copy link
Contributor

@estroz: Overrode contexts on behalf of estroz: coverage/coveralls

In response to this:

/override coverage/coveralls

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.

@k8s-ci-robot k8s-ci-robot merged commit cc270dd into kubernetes-sigs:master Aug 18, 2021
@swiftslee
Copy link
Contributor Author

@estroz thanks for the review. BTW, how do we trigger rebuilding the books? The website is still not updated.

@estroz
Copy link
Contributor

estroz commented Aug 19, 2021

@yuswift typically on each release, the book-v3 branch gets updated. In this case it should be updated sooner. I'll get on that soon.

ping @camilamacedo86

@rashmigottipati
Copy link
Contributor

@estroz I can help with that (if you'd like). Let me know what needs to be done to update the book.

@swiftslee swiftslee deleted the fix_quickstart_prerequisites branch August 20, 2021 02:11
rashmigottipati pushed a commit to rashmigottipati/kubebuilder that referenced this pull request Aug 20, 2021
* update go prerequisites for the latest release

Signed-off-by: yuswift <yuswift2018@gmail.com>

* remove the max goversion limit

Signed-off-by: yuswift <yuswift2018@gmail.com>

* Update docs/book/src/quick-start.md

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>
k8s-ci-robot pushed a commit that referenced this pull request Aug 21, 2021
* update go prerequisites for the latest release

Signed-off-by: yuswift <yuswift2018@gmail.com>

* remove the max goversion limit

Signed-off-by: yuswift <yuswift2018@gmail.com>

* Update docs/book/src/quick-start.md

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>

Co-authored-by: Swift <yuswift2018@gmail.com>
Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>
@rashmigottipati
Copy link
Contributor

@yuswift
#2308 (cherry pick PR of doc fix was merged into the book-v3 branch), so the quickstart prerequisites in the website are now updated to below.

Screen Shot 2021-08-23 at 7 49 45 PM

@swiftslee
Copy link
Contributor Author

@yuswift
#2308 (cherry pick PR of doc fix was merged into the book-v3 branch), so the quickstart prerequisites in the website are now updated to below.

Screen Shot 2021-08-23 at 7 49 45 PM

thanks!

camilamacedo86 pushed a commit that referenced this pull request Aug 4, 2024
* update go prerequisites for the latest release

Signed-off-by: yuswift <yuswift2018@gmail.com>

* remove the max goversion limit

Signed-off-by: yuswift <yuswift2018@gmail.com>

* Update docs/book/src/quick-start.md

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>

Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>

Co-authored-by: Swift <yuswift2018@gmail.com>
Co-authored-by: Eric Stroczynski <ericstroczynski@gmail.com>
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. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
6 participants