-
Notifications
You must be signed in to change notification settings - Fork 47
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
chore: Add script to sync common-instancetypes #900
chore: Add script to sync common-instancetypes #900
Conversation
e46ca25
to
19ffc55
Compare
/cc 0xFelix |
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.
/approve
/hold Just want to add the same |
This change copies logic previously held in the periodic-update-common-instancetypes-bundles job into the SSP repo. This will allow us to remove the duplicate logic from the job definition in the future, simplifying it greatly. Signed-off-by: Lee Yarwood <lyarwood@redhat.com>
19ffc55
to
2dde7df
Compare
Quality Gate passedIssues Measures |
/hold cancel @0xFelix can you take another look? |
|
||
function latest_version() { | ||
if [[ $target_branch == "main" ]]; then | ||
curl --fail -s "https://api.github.com/repos/kubevirt/common-instancetypes/releases/latest" | |
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.
Drop use of --fail
, likely has no effect here?
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.
https://github.com/kubevirt/kubevirt/blob/0ca03aa05ccab5fefc6bfe7b474e5e05e18929a4/hack/bump-common-instancetypes.sh#L8-L14 - That's copied from your script in kubevirt/kubevirt
and I think it does make sense here for the API calls.
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, let's keep it then
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.
/approve
/lgtm
|
||
function latest_version() { | ||
if [[ $target_branch == "main" ]]; then | ||
curl --fail -s "https://api.github.com/repos/kubevirt/common-instancetypes/releases/latest" | |
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, let's keep it then
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 0xFelix 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 |
/cherry-pick release-v0.19 |
@lyarwood: new pull request created: #908 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. |
/cherry-pick release-v0.18 |
@lyarwood: new pull request created: #909 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. |
What this PR does / why we need it:
This change copies logic previously held in the
periodic-update-common-instancetypes-bundles
job into the SSP repo. This will allow us to remove the duplicate logic from the job definition in the future, simplifying it greatly.Which issue(s) this PR fixes:
Partial Fix# kubevirt/common-instancetypes#162
Special notes for your reviewer:
Release note: