-
Notifications
You must be signed in to change notification settings - Fork 469
fix(jenkins): Correct scaffolder action descriptions and parameter handling #5849
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
base: main
Are you sure you want to change the base?
fix(jenkins): Correct scaffolder action descriptions and parameter handling #5849
Conversation
Changed Packages
|
3ed0451 to
fb0214a
Compare
|
Thanks for the contribution! |
|
@04kash Plz review |
fb0214a to
562d281
Compare
|
Hi @awanlin , I have pushed the code but Having trouble getting the checks to pass – the yarn changeset command kept failing with errors, and even after amending with -s, the DCO check isn't clearing. Can you take a look and help me with whats wrong ? |
Heya 👋 It looks like your initial commit was missing the DCO and all commits must have it If you check the DCO status it will link you to some info on how to fix it 😁 sorry I'm on mobile so it's a bit tricky to provide the links! |
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 the contribution @Ayushmore1214, can you please update the title of this PR to be more accurately describe the change. Also, it's really helpful to include that in the body as well. There's no changeset here but that is needed, details on adding one can be found here: https://github.com/backstage/community-plugins/blob/main/CONTRIBUTING.md#creating-changesets
Also, the DCO is still failing.
| "cross-fetch": "^4.0.0", | ||
| "jenkins": "^1.1.0" | ||
| "jenkins": "^1.1.0", | ||
| "zod": "^4.1.12" |
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.
Why do we need zod added 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.
Ah, good point! Since the schema was already using that z => ... syntax (which comes from Zod), I added it as an explicit dependency just to be safe and maybe make things easier if the schema needs more complex stuff later. But definitely happy to remove it if you prefer
| @@ -1,5 +1,5 @@ | |||
| /* | |||
| * Copyright 2024 The Backstage Authors | |||
| * Copyright 2025 The Backstage Authors | |||
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 don't update these dates, the idea is they reflect when this code was added. 👍
| jobName: z.string().describe('Name of the Jenkins job to run'), | ||
| jobParameters: z.record(z.any()).optional(), | ||
| delay: z.number().optional(), | ||
| token: z.string().optional(), |
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.
Where are these two new items coming from - delay and token?
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 saw those mentioned in the original issue (#4871) under Provide the context as possible parameters from the jenkins library, which is why I initially added them to the schema. But you're right, they aren't the main focus of this issue (which is fixing the jobParameters documentation/passing and the enableJob description). I'll remove delay and token from the schema to keep this PR focussed
| async handler(ctx) { | ||
| ctx.logger.info(`Starting jenkins job ${ctx.input.jobName}`); | ||
|
|
||
| async handler(ctx) { |
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.
Please keep existing whitespace and avoid making changes that aren't needed for the task at hand. 👍
Signed-off-by: Ayush More <[email protected]>
Signed-off-by: Ayush More <[email protected]>
Fixes parameter passing for buildJob action, updates enableJob description, corrects README example, and adds changeset. Closes backstage#4871. Signed-off-by: Ayush More <[email protected]>
eeedc8a to
1d42a06
Compare
|
@awanlin Made the changes plz review and let me know for any mistakes or typos , Thank you ! |
Hey, I just made a Pull Request!
This PR addresses issue #4871 regarding documentation and functionality inconsistencies in the Jenkins scaffolder backend module.
I corrected how parameters are passed to the buildJob action in build.ts so it works as expected. I also changed the misleading Destroy description for the enableJob action in enable.ts to correctly say Enable.
✔️ Checklist
Signed-off-byline in the message. (more info)