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

Cleanup CI config #355

Merged
merged 1 commit into from
Jan 31, 2023
Merged

Cleanup CI config #355

merged 1 commit into from
Jan 31, 2023

Conversation

olivierlemasle
Copy link
Member

@olivierlemasle olivierlemasle commented Jan 6, 2023 •
edited
Loading

  • Update GitHub actions
  • Remove duplicated entries in Go versions matrix:
    • '1.x' currently resolves to 'go1.19.4', just like '1.19'
    • stable=false option (which was removed in actions/setup-go@v3) enabled the installation of pre-release Go versions (now possible without that option), but it does not install "tip" version of Go
    • Jobs '1.x' with gostable=true and gostable=false were therefore duplicates of '1.19'
  • Use default checkout path and default GOPATH

@k8s-ci-robot k8s-ci-robot requested review from philips and sttts January 6, 2023 17:18
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jan 6, 2023
@Jefftree
Copy link
Member

Jefftree commented Jan 9, 2023

Thank you for fixing this!

cc @apelisse, I don't have as much context around the 1.x test. When go 1.20 is released, I'm assuming 1.x will default to the latest version? Wouldn't it still be useful to have that test around?

@olivierlemasle
Copy link
Member Author

If we want to always test with the latest Go version, we can also use stable and oldstable aliases (they currently resolve to the latest 1.19.x and the latest 1.18.x version, but they'll be updated automatically when Go 1.20 is released)

apelisse
apelisse reviewed Jan 10, 2023
View reviewed changes
.github/workflows/ci.yml Outdated
include:
- go: '1.x'
gostable: false
go: [ '1.19', '1.18']
Copy link
Member

Choose a reason for hiding this comment

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

nit, but spaces are inconsistent. I agree with Jeff that I like having 1.x automatically pick the newest version. I'm not a big fan of using stable or oldstable since they don't necessarily match the policy that we have. Can we revert that line specifically?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I reverted that line.

I also don't like stable and oldstable very much; I prefer having control on the Go versions used by CI.
However, by keeping both 1.19 and 1.x, we're wasting CI compute resource 🌱

@olivierlemasle
Cleanup CI config
129c723 
- Update GitHub actions
- Remove duplicated entries in Go versions matrix:
  - `stable=false` option (which was removed in actions/setup-go@v3) enabled
    the installation of pre-release Go versions (now possible without
    that option), but it does not install "tip" version of Go
  - Jobs '1.x' with gostable=true and gostable=false were therefore
    duplicates of '1.19'
- Use default checkout path and default GOPATH
@Jefftree
Copy link
Member

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Jefftree, olivierlemasle

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 Jan 24, 2023
@Jefftree
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 31, 2023
@k8s-ci-robot k8s-ci-robot merged commit 5e46c6e into kubernetes:master Jan 31, 2023
@olivierlemasle olivierlemasle deleted the ci branch February 1, 2023 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apelisse apelisse apelisse left review comments

@philips philips Awaiting requested review from philips

@sttts sttts Awaiting requested review from sttts

Assignees

@Jefftree Jefftree

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/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@olivierlemasle @Jefftree @k8s-ci-robot @apelisse