resolve #232 use DataBoundSetter for optional params

[sheehan]

Modify SlackNotifier to use DataBoundSetter instead of constructor params. This resolves #232.

See also https://groups.google.com/forum/#!topic/job-dsl-plugin/JWO7zDuMYVA

[samrocketman]

Seems like a relatively straight forward change. I'll leave this open for review for others to comment.

[zafarella]

pls merge

[daspilker]

@kmadel can you merge this please? I want to drop the built-in support for the Slack plugin in the next release of the Job DSL plugin. Without @DataBoundSetter, the DSL needs to consider all options as mandatory. This would be a major setback for DSL users as the need to inflate their script by adding many options with their default vault.

[daspilker]

@sheehan can you ask on the Jenkins dev list for commit access to this repo to get this merged?

[MichaelPereira]

This change would be welcome, please consider asking for review or commit access to https://groups.google.com/forum/#!forum/jenkinsci-dev @sheehan, thanks ! 😄

[kmadel]

I will be happy to merge once the conflict is resolved and there are two thumbs up for a code review.

[sheehan]

rebased

[daspilker]

looks good 👍

[fgs-dbudwin]

Any progress on this? Looks a like another rebase is in order @sheehan

[sheehan]

up to date

[samrocketman]

I'll review this next week and merge it if nothing is done by then. Currently, other life events have my attn this week.

[samrocketman]

I just upgraded my automated scripts for bootstrapping Jenkins configured with slack. https://github.com/samrocketman/jenkins-bootstrap-slack I'm going to test this today and if all checks out I'll merge it.

[samrocketman]

Tested and works just fine.

[daspilker]

Thanks for merging this. Are you planning to cut a release anytime soon?

[samrocketman]

Yeah, I'm trying to get a few more PRs rebased/merge. However, I'm willing to commit to a release by this coming weekend.

[Daniel15]

Thanks for this pull request. I'm not using the Slack plugin, but this helped me understand the exact same issue with another plugin.

@daspilker - Is the fact that plugins should use DataBoundSetters for Job DSL documented anywhere?

[daspilker]

@Daniel15 use DataBoundConstructor for mandatory settings and DataBoundSetter for optional settings.

[Daniel15]

@daspilker - Sure, I understand that now, but is it mentioned in documentation anywhere (either Jenkins docs or Job DSL docs)? 😃

[samrocketman]

@daspilker feel free to subscribe to #296 for release updates.