Parameterized Generators and Pipelines #2132
Conversation
M-Davies
added
enhancement
M-Davies
force-pushed
the
parameterised_everything
branch
3 times, most recently
from
3d24d99 to
2310bfd
Compare
M-Davies
force-pushed
the
parameterised_everything
branch
2 times, most recently
from
e4f7d28 to
27a980f
Compare
|
https://ci.adoptopenjdk.net/job/build-scripts/job/utils/job/alpha-build-pipeline-generator/49/console proves this works with custom values and repository (the test jobs were created in wip: https://ci.adoptopenjdk.net/view/work-in-progress/job/build-scripts-testing/). I'll revert the file changes and continue testing |
M-Davies
force-pushed
the
parameterised_everything
branch
2 times, most recently
from
fae008a to
1377ad7
Compare
pipelines/build/regeneration/jdk10_regeneration_pipeline.groovy
Outdated
Show resolved
Hide resolved
docs/UsingOurScripts.md
Outdated
| ```md | ||
| 1. JENKINS PARAMETERS (highest priority, args entered here will be what the build scripts use over everything else) | ||
| 2. USER JSON (medium priority, args entered here will be used when a jenkins parameter isn't entered) | ||
| 3. ADOPT JSON (final priority, when jenkins parameters AND a user json arg can't be validated, the script will checkout to this repository and use our defaults json (linked above)) |
There was a problem hiding this comment.
Is this the right order? I'd have thought that the user over ride would be the last one.
There was a problem hiding this comment.
User override should ideally be the first otherwise the scripts would just pull in Adopt's data no questions asked. Working vice versa would defeat the point of the PR and Adam's review above.
If a user doesn't want to use Adopt's data and wants to use their own, they should follow the instructions in the guide and either point their defaults.json at whatever the filepath to their data is and leave the Jenkins parameters alone OR stick the path in the Jenkins parameters and leave the defaults.json alone.
Vice versa, if a user wants to use Adopt's data and not have the jobs detect their own data (if they have it), they should remove or invalidate the corresponding entry in their defaults.json and the scripts will automatically pull and use Adopt's scripts.
Happy to discuss this further or validate any points that are confusing? I recognise it's a lot of changes and a 180 degree behaviour in terms of the pipelines 🙂
There was a problem hiding this comment.
Let's have a call - I get what the intention is here, it just goes slightly against the typical design for these sorts of systems where the defaults are laid ontop of each other (and are well documented and disseminated!) and the user overrides come last.
There was a problem hiding this comment.
The final decision has been to open up a follow up issue to address expanding the current USE_ADOPT_SHELL_SCRIPTS parameter to cover all instances of the checking out (see checklist comment).
This will likely require refactoring and upgrading once the repo strip out goes through so we will need to keep that in mind after this is merged
There was a problem hiding this comment.
|
AQA meeting recording of me discussing these changes (start at 3:05) -> https://youtu.be/Fql05uEF9ZA?t=185 |
|
I'm afraid a rebase will also be required now sorry! |
|
Thanks for taking this on @M-Davies (and for walking us through it, re: #2132 (comment)). |
|
Update thread for addressing issues and the checklist comment above -> https://adoptopenjdk.slack.com/archives/C09NW3L2J/p1612435163113400 |
Closes: #2116
Signed-off-by: Morgan Davies morgandavies2020@gmail.com