Skip to content

Make some settings optional in pipelines #119

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

Merged
merged 2 commits into from
Jul 21, 2019

Conversation

proski
Copy link

@proski proski commented Jul 11, 2019

All advanced settings have reasonable defaults so they can be optional settings. Move then from @DataBoundConstructor to individual setters.

Make StashBuildTrigger initialize fields to the config.jelly defaults. That way, the Pipeline Snippet Generator knows what the defaults are and skips them from the snippets.

A nice side effect is having test this please and NO TEST shared between the code and config.jelly.

This PR is based on top of #69, as it's untestable without pipeline support.

The changes are based on https://jenkins.io/doc/developer/plugin-development/pipeline-integration/

@jakub-bochenski
Copy link

jakub-bochenski commented Jul 11, 2019

This PR is based on top of #69, as it's untestable without pipeline support.

The part that moves fields into @DataBoundSetter is actually beneficial to the job-dsl plugin usage. I don't think job-dsl cares about jelly though.
Could you maybe split this change to a separate PR so that it can be checked against dynamic dsl?

@proski proski force-pushed the optional-settings branch from 594f945 to eadd996 Compare July 11, 2019 16:19
@proski
Copy link
Author

proski commented Jul 11, 2019

OK, I removed the dependency on pipelines (#69) but I left the dependency on #116. Actually, I hope both will be merged soon.

Change the constructor to accept essential settings only.

Reorder private fields, setters and getters the way they are ordered in
config.jelly.
@proski proski force-pushed the optional-settings branch 3 times, most recently from 729a636 to 61284bd Compare July 12, 2019 21:26
That makes the Pipeline Snippet Generator skip arguments from the trigger
call if those arguments have default values.

Keep default values for the skip phrase and the build phrase in the
descriptor, so they can be used both in config.jelly and in the code.
@proski proski force-pushed the optional-settings branch from 61284bd to 48609f7 Compare July 14, 2019 02:59
@jakub-bochenski jakub-bochenski merged commit 5aae7f6 into jenkinsci:master Jul 21, 2019
@proski proski deleted the optional-settings branch July 21, 2019 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants