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

change kubectl download site #1007

Merged
merged 2 commits into from
Feb 24, 2024
Merged

change kubectl download site #1007

merged 2 commits into from
Feb 24, 2024

Conversation

YoninL
Copy link
Contributor

@YoninL YoninL commented Dec 26, 2023

Change kubectl download site from https://storage.googleapis.com/kubernetes-release/release to https://dl.k8s.io

Description

Change kubectl download site from https://storage.googleapis.com/kubernetes-release/release to https://dl.k8s.io

Motivation and Context

How Has This Been Tested?

get_test.go is updated.

If updating or adding a new CLI to arkade get, run:

go build && ./hack/test-tool.sh TOOL_NAME

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Documentation

  • I have updated the list of tools in README.md if (required) with ./arkade get --format markdown
  • I have updated the list of apps in README.md if (required) with ./arkade install --help

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I've read the CONTRIBUTION guide
  • I have signed-off my commits with git commit -s
  • I have tested this on arm, or have added code to prevent deployment

Signed-off-by: YoninL <jiayo125@gmail.com>
Signed-off-by: YoninL <jiayo125@gmail.com>
Copy link
Contributor

@Jasstkn Jasstkn left a comment

Choose a reason for hiding this comment

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

LGTM.

@YoninL
Copy link
Contributor Author

YoninL commented Jan 28, 2024

@Jasstkn Thank you, can you help to merge this PR? I don't have write permission, thanks.

Copy link
Owner

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved

@alexellis
Copy link
Owner

@Jasstkn I'm assuming you've tested this and that's why you approved the PR. I'm merging it, but I haven't tested it or verified it in any way.

Hope you're OK with sending a PR to revert this if there's an issue and breakage for users?

@alexellis
Copy link
Owner

@YoninL has also provided no evidence of testing, he changed this in the UI as you can see from "patch-1"

@alexellis alexellis merged commit 1d762c3 into alexellis:master Feb 24, 2024
@alexellis
Copy link
Owner

@Jasstkn I merged this and did a release, it broke the build and we have a faulty publish:

https://github.com/alexellis/arkade/actions/runs/8029701263/job/21936281148

alexellis added a commit that referenced this pull request Feb 24, 2024
Signed-off-by: Alex Ellis (OpenFaaS Ltd) <alexellis2@gmail.com>
@YoninL YoninL deleted the patch-1 branch February 24, 2024 14:11
@YoninL
Copy link
Contributor Author

YoninL commented Feb 24, 2024

Thanks for fixing the issue, yes, was updating in the UI, sorry I missed some links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants