-
-
Notifications
You must be signed in to change notification settings - Fork 72
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
fix: rename argument parameters #440
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: sakshi1215 <[email protected]>
Thank you for creating a pull request!Please check out the information below if you have not made a pull request here before (or if you need a reminder how things work). Code Quality and Contributing GuidelinesIf you have not done so already, please familiarise yourself with our Contributing Guidelines and Code Of Conduct, even if you have contributed before. TestsGithub actions will run a set of jobs against your PR that will lint and unit test your changes. Keep an eye out for the results from these on the latest commit you submitted. For more information, please see our testing documentation. In order to run the advanced pipeline tests (executing a set of mock pipelines), it requires an admin to post |
Hi @zdtsw can you please review this? |
Signed-off-by: sakshi1215 <[email protected]>
Ping @AdamBrousseau to double check if these two parameters need to be updated for your Jenkins job |
Is there a manual update needed to job(s) or will they automatically regenerate with the updated names? |
|
@karianna @zdtsw @AdamBrousseau Is this good to be merged? |
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.
Approving from a Semeru perspective.
run tests |
PR TESTER RESULT ❎ Some pipelines failed or the job was aborted! ❎ |
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.
my bad, should have considered this rename from CLEAN_WORKSPACE
to CLEAN_WORKSPACE_BEFORE
would require the change in the class IndividualBuildConfig, which is not in the scope of this git repo.
Can I tru that change too? Which repo stored the buildConfig?
…On Wed, 12 Oct 2022 at 10:19 PM, Wen Zhou ***@***.***> wrote:
***@***.**** commented on this pull request.
my bad, should have consider this change would require the change in the
class IndividualBuildConfig, which is not in the scope of this git repo.
—
Reply to this email directly, view it on GitHub
<#440 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ANF3CEDX2LQ2Y5ZWAD4MC4LWC3TYNANCNFSM6AAAAAARBIOUII>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
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.
This is good but needs to go in with corresponding fix in the other repo
@karianna @zdtsw Here is the PR adoptium/jenkins-helper#43 |
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 okay, but not sure we should merge just before a release?
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.
Actually we'd need to change this as well: https://github.com/adoptium/jenkins-helper/blob/9fd063790c64a7cf2f9ee38c4bf4655f430d8be6/src/common/IndividualBuildConfig.groovy#L58
Its already done @andrew-m-leonard adoptium/jenkins-helper#43 |
excellent |
Will merge with the jenkins helper fix post release. |
fixes #433
Signed-off-by: sakshi1215 [email protected]