-
Notifications
You must be signed in to change notification settings - Fork 891
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
Only update krew index for latest stable version. #5224
Only update krew index for latest stable version. #5224
Conversation
88e63a1
to
34e2cf2
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #5224 +/- ##
==========================================
+ Coverage 31.14% 31.47% +0.33%
==========================================
Files 640 643 +3
Lines 44417 44436 +19
==========================================
+ Hits 13833 13987 +154
+ Misses 29583 29423 -160
- Partials 1001 1026 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@liangyuanpeng What a coincidence, we must have been on the same wavelength! |
34e2cf2
to
26da63c
Compare
ping @zhzhuang-zju for review :) |
/assign |
/lgtm |
b68cc77
to
3975092
Compare
.github/workflows/release.yml
Outdated
update-krew-index: | ||
needs: release-assests | ||
needs: | ||
- release-assests | ||
- get-stable-tag | ||
if: needs.get-stable-tag.outputs.stableTag == github.event.release.tag_name |
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.
Is it possible to get the latest tag at the update-krew-index
job? So that we don't need another job.
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.
krew-release-bot
action don't care the version #5224 (comment)
It is possible to do this using the krew-release-bot
binary instead of the krew-release-bot
action https://github.com/rajatjindal/krew-release-bot/blob/ad6e92e6daadabfedaa107ce5186a82d41229ef1/examples/circleci.yml#L17-L18, but that means we'll be dropping the automatic updates for rajatjindal/krew-release-bot
action, I'd update it if that's what you want.
then it will be:
update-krew-index:
gh api....
// check is it the latest version
curl -LO https://github.com/rajatjindal/krew-release-bot/releases/download/${KREW_RELEASE_BOT_VERSION}/krew-release-bot_${KREW_RELEASE_BOT_VERSION}_linux_amd64.tar.gz
tar -xvf krew-release-bot_${KREW_RELEASE_BOT_VERSION}_linux_amd64.tar.gz
./krew-release-bot action
But I'd go with adding a job, it's not a big deal.
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.
We can wait and see how upstream reply.
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 didn't mean to hack anything of rajatjindal/krew-release-bot
, I mean adding an extra step to the update-krew-index
job, so that we don't need to share the variable between jobs.
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.
Thanks for your clarify, i have update it , add a step and not a new job.
NOTE: have not test yet.
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.
Looks much better now.
.github/workflows/release.yml
Outdated
- uses: actions/checkout@v4 | ||
if: steps.check-latest.outputs.stableTag == github.event.release.tag_name |
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.
Maybe we don't need to check out the code.
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.
it's not true, krew-release-bot need the file of https://github.com/karmada-io/karmada/blob/master/.krew.yaml , https://github.com/rajatjindal/krew-release-bot?tab=readme-ov-file#basic-setup
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.
OK, then keep it.
5d6e5e8
to
939c1a4
Compare
Signed-off-by: Lan Liang <gcslyp@gmail.com>
0d82401
to
31177dd
Compare
/retest |
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
/approve
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RainbowMango 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 |
- uses: actions/checkout@v4 | ||
if: steps.get-latest-tag.outputs.latestTag == github.event.release.tag_name | ||
- name: Update new version in krew-index | ||
if: steps.get-latest-tag.outputs.latestTag == github.event.release.tag_name |
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.
The two steps were skipped, I don't know if this workflow could get the right latest tag, maybe there is a syntax error?
https://github.com/karmada-io/karmada/actions/runs/10645084310/job/29510560441#step:3:3
I can get the latest tag on my side.
# gh api repos/karmada-io/karmada/releases/latest | jq -r '.tag_name'
v1.11.0
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
Only update krew index for latest stable version. Avoid sending noisy PRs and in the future,we don't need some PR anymore like:
/cc @RainbowMango @zhzhuang-zju
Which issue(s) this PR fixes:
Ref #4703
Fixes #
Special notes for your reviewer:
Tested on my fork repo:
Update krew index
)Update krew index
due to karmada latest stable version is v1.10.2 and not v1.99.3 )Does this PR introduce a user-facing change?: