StashBuildTrigger: Use correct getter names for config.jelly fields

[proski]

Getter names for fields referenced in config.jelly should start with "get" followed by the capitalized property name. "is" is not supported.

That's an alternative to #72 and it has unit tests.

It's not a pipeline only issue and should not be blocked by any other PR. The current code is not following the rule recommended in the Jenkins Wiki.

https://wiki.jenkins.io/display/JENKINS/Basic+guide+to+Jelly+usage+in+Jenkins#BasicguidetoJellyusageinJenkins-Understandingtheitobject

[jakub-bochenski]

The documentation you link to isn't very clear.

I'd recommend using the Java convention for naming methods (e.g. starting getters with 'get' and using CamelCase) so that Jelly can always find the methods.

is prefix for boolean properties is also part of the established Java naming convention. The get part is only an example (implying there are other possibilities).

In this light I appreciate adding the Descriptor unit test -- at least we have automated check that Jenkins recognizes the properties (whatever the logic for recognizing them might be).

Still -- I'm worried about job-dsl. Changing method names could cause problems with the autogenerated DSL API.

[jakub-bochenski]

FYI according to build StashBuildTriggerTest has wrong formatting now

[jakub-bochenski]

Still -- I'm worried about job-dsl. Changing method names could cause problems with the autogenerated DSL API.

We are safe with the getter/setter changes because we use @DataBoundConstructor and not the setter annotations.

[proski]

Still -- I'm worried about job-dsl. Changing method names could cause problems with the autogenerated DSL API.

We are safe with the getter/setter changes because we use @DataBoundConstructor and not the setter annotations.

We should move optional parameters to setters according to this document:
https://jenkins.io/doc/developer/plugin-development/pipeline-integration/

[proski]

is prefix for boolean properties is also part of the established Java naming convention. The get part is only an example (implying there are other possibilities).

I can send a patch for Jenkins to recognize "is", but it will be a long time until we can require the new Jenkins version.

Still -- I'm worried about job-dsl. Changing method names could cause problems with the autogenerated DSL API.

That's another reason we should move to individual setters for parameters with defaults. The generated DSL won't include default settings.

[jakub-bochenski]

I can send a patch for Jenkins to recognize "is", but it will be a long time until we can require the new Jenkins version.

I'm not asking for this, just wanted to explain why I think this change isn't as trivial as it might seem

That's another reason we should move to individual setters for parameters with defaults. The generated DSL won't include default settings.

Agree

[jakub-bochenski]

@guyv any comments? for some reason I can't add you as a reviewer

[guyv]

@guyv any comments? for some reason I can't add you as a reviewer

@jakub-bochenski looks good to me