Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

Ok, so that people can see code rather than just yapping on the M-L (and
given that we seem to be OK with the annotation names)

https://github.com/stapler/stapler/pull/121

Is where I am working on this proposal. We can continue the discussion in
either place as appropriate (general comments here, specific comments on
specific details of the code in the PR)

On 7 July 2017 at 03:06, Stephen Connolly 
wrote:

> Yes I was thinking that a custom javadoc handler would be useful.
> Especially as there are inheritance of facets and fragments that we'd want
> to see documented correctly.
>
> On 7 July 2017 at 02:37, Robert Sandell  wrote:
>
>> I agree with Stephen here; adding methods seems risky and could break
>> existing plugins, at least in a backwards compat sense.
>>
>> But I at the same time do like the idea of getting javadoc for the facets
>> and fragments. So can we get both somehow?
>> E.g. if there is a @StaplerFacet("login") or @StaplerFragment("main") there
>> should be a corresponding
>>
>> @facet login the login page.
>> @fragment main add this to change the main part of the configuration page.
>>
>> in the javadoc of the class?
>>
>> /B
>>
>> 2017-07-06 13:16 GMT+02:00 Stephen Connolly <
>> stephen.alan.conno...@gmail.com>:
>>
>>>
>>>
>>> On 5 July 2017 at 18:10, Kohsuke Kawaguchi  wrote:
>>>
 FWIW, I've already discussed this offline with Stephen and I'm
 generally +1.

 One suggestion I had for him is to consider using a Java method
 definition instead of @StaplerFacet and @StaplerFragment. That is, instead
 of this:

 @StaplerFacets({
   @StaplerFacet("index"),
   @StaplerFacet("config")
 })
 @StaplerFragments({
   @StaplerFragment("main"),
   @StaplerFragment("side-panel"),
   @StaplerFragment(value="tasks",optional=true)
 })
 public class Widget { ... }

 Consider that:

 public class Widget {
 @StaplerFacet protected void index() {}
 @StaplerFacet protected void config() {}

 @StaplerFragment protected abstract void main();
 @StaplerFragment("side-panel") protected abstract void sidepanel();
 @StaplerFragment protected void tasks();
 }


>>> Well at least this is a better explanation than you gave me verbally ;-)
>>>
>>>
 I liked this better because ...

- It fits with the original design thinking behind Stapler, which
is that views are like methods. Their inheritance and override semantics
are all modeled after methods.
- It avoids a long list of @StaplerFragment/@StaplerFacet at the
top of the class declaration. We have some objects in the core that has
quite a few views.

 So let's see:
>>>
>>> $ ( cd core/src/main/resources ; find . -name \*.jelly -exec dirname {}
>>> \; ; find . -name \*.groovy -exec dirname {} \; ) | sort | uniq -c | grep
>>> -v ./lib/ | sort -n | tail -n 10
>>>7 ./hudson/model/AbstractProject
>>>7 ./hudson/slaves/SlaveComputer
>>>7 ./jenkins/install/SetupWizard
>>>9 ./hudson/model/Run
>>>   11 ./hudson/PluginManager
>>>   11 ./hudson/security/HudsonPrivateSecurityRealm
>>>   12 ./hudson/model/Job
>>>   13 ./hudson/model/View
>>>   14 ./hudson/model/Computer
>>>   25 ./jenkins/model/Jenkins
>>>
>>> So there are not altogether too many classes in Jenkins having lots of
>>> fragments / facets
>>>
>>> I think an argument based on a couple of really big classes like Jenkins
>>> having 25 lines of
>>>
>>> @StaplerObject
>>> @StaplerFacet("_restart")
>>> @StaplerFacet("_safeRestart")
>>> @StaplerFacet("_script")
>>> @StaplerFacet("_scriptText")
>>> @StaplerFacet("accessDenied")
>>> @StaplerFacet("configure")
>>> @StaplerFacet("configureExecutors")
>>> @StaplerFacet("fingerprintCheck")
>>> @StaplerFacet("legend")
>>> @StaplerFacet("load-statistics")
>>> @StaplerFacet("login")
>>> @StaplerFacet("loginError")
>>> @StaplerFacet("manag_")
>>> @StaplerFacet("manage")
>>> @StaplerFacet("newView")
>>> @StaplerFacet("noPrincipal")
>>> @StaplerFacet("oops")
>>> @StaplerFacet("opensearch.xml")
>>> @StaplerFacet("projectRelationship-help")
>>> @StaplerFacet("projectRelationship")
>>> @StaplerFacet("systemInfo")
>>> @StaplerFacet("threadDump")
>>> @StaplerFragment("_api")
>>> @StaplerFragment("downgrade")
>>> @StaplerFragment("sidepanel")
>>> public class Jenkins {
>>>   ...
>>> }
>>>
>>> compared with
>>>
>>> @StaplerObject
>>> public class Jenkins {
>>>   @StaplerFacet protected void _restart() {}
>>>   @StaplerFacet protected void _safeRestart() {}
>>>   @StaplerFacet protected void _script() {}
>>>   @StaplerFacet protected void _scriptText() {}
>>>   @StaplerFacet protected void accessDenied() {}
>>>   @StaplerFacet protected void configure() {}
>>>   @StaplerFacet protected void configureExecutors() {}
>>>   @StaplerFacet protected void fingerprintCheck() {}
>>>   @StaplerFacet protected 

Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

Yes I was thinking that a custom javadoc handler would be useful.
Especially as there are inheritance of facets and fragments that we'd want
to see documented correctly.

On 7 July 2017 at 02:37, Robert Sandell  wrote:

> I agree with Stephen here; adding methods seems risky and could break
> existing plugins, at least in a backwards compat sense.
>
> But I at the same time do like the idea of getting javadoc for the facets
> and fragments. So can we get both somehow?
> E.g. if there is a @StaplerFacet("login") or @StaplerFragment("main") there
> should be a corresponding
>
> @facet login the login page.
> @fragment main add this to change the main part of the configuration page.
>
> in the javadoc of the class?
>
> /B
>
> 2017-07-06 13:16 GMT+02:00 Stephen Connolly  com>:
>
>>
>>
>> On 5 July 2017 at 18:10, Kohsuke Kawaguchi  wrote:
>>
>>> FWIW, I've already discussed this offline with Stephen and I'm
>>> generally +1.
>>>
>>> One suggestion I had for him is to consider using a Java method
>>> definition instead of @StaplerFacet and @StaplerFragment. That is, instead
>>> of this:
>>>
>>> @StaplerFacets({
>>>   @StaplerFacet("index"),
>>>   @StaplerFacet("config")
>>> })
>>> @StaplerFragments({
>>>   @StaplerFragment("main"),
>>>   @StaplerFragment("side-panel"),
>>>   @StaplerFragment(value="tasks",optional=true)
>>> })
>>> public class Widget { ... }
>>>
>>> Consider that:
>>>
>>> public class Widget {
>>> @StaplerFacet protected void index() {}
>>> @StaplerFacet protected void config() {}
>>>
>>> @StaplerFragment protected abstract void main();
>>> @StaplerFragment("side-panel") protected abstract void sidepanel();
>>> @StaplerFragment protected void tasks();
>>> }
>>>
>>>
>> Well at least this is a better explanation than you gave me verbally ;-)
>>
>>
>>> I liked this better because ...
>>>
>>>- It fits with the original design thinking behind Stapler, which is
>>>that views are like methods. Their inheritance and override semantics are
>>>all modeled after methods.
>>>- It avoids a long list of @StaplerFragment/@StaplerFacet at the top
>>>of the class declaration. We have some objects in the core that has 
>>> quite a
>>>few views.
>>>
>>> So let's see:
>>
>> $ ( cd core/src/main/resources ; find . -name \*.jelly -exec dirname {}
>> \; ; find . -name \*.groovy -exec dirname {} \; ) | sort | uniq -c | grep
>> -v ./lib/ | sort -n | tail -n 10
>>7 ./hudson/model/AbstractProject
>>7 ./hudson/slaves/SlaveComputer
>>7 ./jenkins/install/SetupWizard
>>9 ./hudson/model/Run
>>   11 ./hudson/PluginManager
>>   11 ./hudson/security/HudsonPrivateSecurityRealm
>>   12 ./hudson/model/Job
>>   13 ./hudson/model/View
>>   14 ./hudson/model/Computer
>>   25 ./jenkins/model/Jenkins
>>
>> So there are not altogether too many classes in Jenkins having lots of
>> fragments / facets
>>
>> I think an argument based on a couple of really big classes like Jenkins
>> having 25 lines of
>>
>> @StaplerObject
>> @StaplerFacet("_restart")
>> @StaplerFacet("_safeRestart")
>> @StaplerFacet("_script")
>> @StaplerFacet("_scriptText")
>> @StaplerFacet("accessDenied")
>> @StaplerFacet("configure")
>> @StaplerFacet("configureExecutors")
>> @StaplerFacet("fingerprintCheck")
>> @StaplerFacet("legend")
>> @StaplerFacet("load-statistics")
>> @StaplerFacet("login")
>> @StaplerFacet("loginError")
>> @StaplerFacet("manag_")
>> @StaplerFacet("manage")
>> @StaplerFacet("newView")
>> @StaplerFacet("noPrincipal")
>> @StaplerFacet("oops")
>> @StaplerFacet("opensearch.xml")
>> @StaplerFacet("projectRelationship-help")
>> @StaplerFacet("projectRelationship")
>> @StaplerFacet("systemInfo")
>> @StaplerFacet("threadDump")
>> @StaplerFragment("_api")
>> @StaplerFragment("downgrade")
>> @StaplerFragment("sidepanel")
>> public class Jenkins {
>>   ...
>> }
>>
>> compared with
>>
>> @StaplerObject
>> public class Jenkins {
>>   @StaplerFacet protected void _restart() {}
>>   @StaplerFacet protected void _safeRestart() {}
>>   @StaplerFacet protected void _script() {}
>>   @StaplerFacet protected void _scriptText() {}
>>   @StaplerFacet protected void accessDenied() {}
>>   @StaplerFacet protected void configure() {}
>>   @StaplerFacet protected void configureExecutors() {}
>>   @StaplerFacet protected void fingerprintCheck() {}
>>   @StaplerFacet protected void legend() {}
>>   @StaplerFacet @StaplerPath("load-statistics") protected void
>> loadStatistics() {}
>>   @StaplerFacet protected void login() {}
>>   @StaplerFacet protected void loginError() {}
>>   @StaplerFacet protected void manag_() {}
>>   @StaplerFacet protected void manage() {}
>>   @StaplerFacet protected void newView() {}
>>   @StaplerFacet protected void noPrincipal() {}
>>   @StaplerFacet protected void oops() {}
>>   @StaplerFacet @StaplerPath("opensearch.xml") protected void
>> opensearchXml() {}
>>   @StaplerFacet @StaplerPath("projectRelationship-help") 

Re: [Proposal] Making all things Stapler more declarative

[Robert Sandell]

I agree with Stephen here; adding methods seems risky and could break
existing plugins, at least in a backwards compat sense.

But I at the same time do like the idea of getting javadoc for the facets
and fragments. So can we get both somehow?
E.g. if there is a @StaplerFacet("login") or @StaplerFragment("main") there
should be a corresponding

@facet login the login page.
@fragment main add this to change the main part of the configuration page.

in the javadoc of the class?

/B

2017-07-06 13:16 GMT+02:00 Stephen Connolly :

>
>
> On 5 July 2017 at 18:10, Kohsuke Kawaguchi  wrote:
>
>> FWIW, I've already discussed this offline with Stephen and I'm
>> generally +1.
>>
>> One suggestion I had for him is to consider using a Java method
>> definition instead of @StaplerFacet and @StaplerFragment. That is, instead
>> of this:
>>
>> @StaplerFacets({
>>   @StaplerFacet("index"),
>>   @StaplerFacet("config")
>> })
>> @StaplerFragments({
>>   @StaplerFragment("main"),
>>   @StaplerFragment("side-panel"),
>>   @StaplerFragment(value="tasks",optional=true)
>> })
>> public class Widget { ... }
>>
>> Consider that:
>>
>> public class Widget {
>> @StaplerFacet protected void index() {}
>> @StaplerFacet protected void config() {}
>>
>> @StaplerFragment protected abstract void main();
>> @StaplerFragment("side-panel") protected abstract void sidepanel();
>> @StaplerFragment protected void tasks();
>> }
>>
>>
> Well at least this is a better explanation than you gave me verbally ;-)
>
>
>> I liked this better because ...
>>
>>- It fits with the original design thinking behind Stapler, which is
>>that views are like methods. Their inheritance and override semantics are
>>all modeled after methods.
>>- It avoids a long list of @StaplerFragment/@StaplerFacet at the top
>>of the class declaration. We have some objects in the core that has quite 
>> a
>>few views.
>>
>> So let's see:
>
> $ ( cd core/src/main/resources ; find . -name \*.jelly -exec dirname {} \;
> ; find . -name \*.groovy -exec dirname {} \; ) | sort | uniq -c | grep -v
> ./lib/ | sort -n | tail -n 10
>7 ./hudson/model/AbstractProject
>7 ./hudson/slaves/SlaveComputer
>7 ./jenkins/install/SetupWizard
>9 ./hudson/model/Run
>   11 ./hudson/PluginManager
>   11 ./hudson/security/HudsonPrivateSecurityRealm
>   12 ./hudson/model/Job
>   13 ./hudson/model/View
>   14 ./hudson/model/Computer
>   25 ./jenkins/model/Jenkins
>
> So there are not altogether too many classes in Jenkins having lots of
> fragments / facets
>
> I think an argument based on a couple of really big classes like Jenkins
> having 25 lines of
>
> @StaplerObject
> @StaplerFacet("_restart")
> @StaplerFacet("_safeRestart")
> @StaplerFacet("_script")
> @StaplerFacet("_scriptText")
> @StaplerFacet("accessDenied")
> @StaplerFacet("configure")
> @StaplerFacet("configureExecutors")
> @StaplerFacet("fingerprintCheck")
> @StaplerFacet("legend")
> @StaplerFacet("load-statistics")
> @StaplerFacet("login")
> @StaplerFacet("loginError")
> @StaplerFacet("manag_")
> @StaplerFacet("manage")
> @StaplerFacet("newView")
> @StaplerFacet("noPrincipal")
> @StaplerFacet("oops")
> @StaplerFacet("opensearch.xml")
> @StaplerFacet("projectRelationship-help")
> @StaplerFacet("projectRelationship")
> @StaplerFacet("systemInfo")
> @StaplerFacet("threadDump")
> @StaplerFragment("_api")
> @StaplerFragment("downgrade")
> @StaplerFragment("sidepanel")
> public class Jenkins {
>   ...
> }
>
> compared with
>
> @StaplerObject
> public class Jenkins {
>   @StaplerFacet protected void _restart() {}
>   @StaplerFacet protected void _safeRestart() {}
>   @StaplerFacet protected void _script() {}
>   @StaplerFacet protected void _scriptText() {}
>   @StaplerFacet protected void accessDenied() {}
>   @StaplerFacet protected void configure() {}
>   @StaplerFacet protected void configureExecutors() {}
>   @StaplerFacet protected void fingerprintCheck() {}
>   @StaplerFacet protected void legend() {}
>   @StaplerFacet @StaplerPath("load-statistics") protected void
> loadStatistics() {}
>   @StaplerFacet protected void login() {}
>   @StaplerFacet protected void loginError() {}
>   @StaplerFacet protected void manag_() {}
>   @StaplerFacet protected void manage() {}
>   @StaplerFacet protected void newView() {}
>   @StaplerFacet protected void noPrincipal() {}
>   @StaplerFacet protected void oops() {}
>   @StaplerFacet @StaplerPath("opensearch.xml") protected void
> opensearchXml() {}
>   @StaplerFacet @StaplerPath("projectRelationship-help") protected void
> projectRelationshipHelp()
>   @StaplerFacet protected void projectRelationship() {}
>   @StaplerFacet protected void systemInfo() {}
>   @StaplerFacet protected void threadDump() {}
>   @StaplerFragment protected void _api() {}
>   @StaplerFragment protected void downgrade() {}
>   @StaplerFragment protected void sidepanel() {}
>   ...
> }
>
> just to get the javadoc comments 

Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

On 5 July 2017 at 18:10, Kohsuke Kawaguchi  wrote:

> FWIW, I've already discussed this offline with Stephen and I'm
> generally +1.
>
> One suggestion I had for him is to consider using a Java method definition
> instead of @StaplerFacet and @StaplerFragment. That is, instead of this:
>
> @StaplerFacets({
>   @StaplerFacet("index"),
>   @StaplerFacet("config")
> })
> @StaplerFragments({
>   @StaplerFragment("main"),
>   @StaplerFragment("side-panel"),
>   @StaplerFragment(value="tasks",optional=true)
> })
> public class Widget { ... }
>
> Consider that:
>
> public class Widget {
> @StaplerFacet protected void index() {}
> @StaplerFacet protected void config() {}
>
> @StaplerFragment protected abstract void main();
> @StaplerFragment("side-panel") protected abstract void sidepanel();
> @StaplerFragment protected void tasks();
> }
>
>
Well at least this is a better explanation than you gave me verbally ;-)


> I liked this better because ...
>
>- It fits with the original design thinking behind Stapler, which is
>that views are like methods. Their inheritance and override semantics are
>all modeled after methods.
>- It avoids a long list of @StaplerFragment/@StaplerFacet at the top
>of the class declaration. We have some objects in the core that has quite a
>few views.
>
> So let's see:

$ ( cd core/src/main/resources ; find . -name \*.jelly -exec dirname {} \;
; find . -name \*.groovy -exec dirname {} \; ) | sort | uniq -c | grep -v
./lib/ | sort -n | tail -n 10
   7 ./hudson/model/AbstractProject
   7 ./hudson/slaves/SlaveComputer
   7 ./jenkins/install/SetupWizard
   9 ./hudson/model/Run
  11 ./hudson/PluginManager
  11 ./hudson/security/HudsonPrivateSecurityRealm
  12 ./hudson/model/Job
  13 ./hudson/model/View
  14 ./hudson/model/Computer
  25 ./jenkins/model/Jenkins

So there are not altogether too many classes in Jenkins having lots of
fragments / facets

I think an argument based on a couple of really big classes like Jenkins
having 25 lines of

@StaplerObject
@StaplerFacet("_restart")
@StaplerFacet("_safeRestart")
@StaplerFacet("_script")
@StaplerFacet("_scriptText")
@StaplerFacet("accessDenied")
@StaplerFacet("configure")
@StaplerFacet("configureExecutors")
@StaplerFacet("fingerprintCheck")
@StaplerFacet("legend")
@StaplerFacet("load-statistics")
@StaplerFacet("login")
@StaplerFacet("loginError")
@StaplerFacet("manag_")
@StaplerFacet("manage")
@StaplerFacet("newView")
@StaplerFacet("noPrincipal")
@StaplerFacet("oops")
@StaplerFacet("opensearch.xml")
@StaplerFacet("projectRelationship-help")
@StaplerFacet("projectRelationship")
@StaplerFacet("systemInfo")
@StaplerFacet("threadDump")
@StaplerFragment("_api")
@StaplerFragment("downgrade")
@StaplerFragment("sidepanel")
public class Jenkins {
  ...
}

compared with

@StaplerObject
public class Jenkins {
  @StaplerFacet protected void _restart() {}
  @StaplerFacet protected void _safeRestart() {}
  @StaplerFacet protected void _script() {}
  @StaplerFacet protected void _scriptText() {}
  @StaplerFacet protected void accessDenied() {}
  @StaplerFacet protected void configure() {}
  @StaplerFacet protected void configureExecutors() {}
  @StaplerFacet protected void fingerprintCheck() {}
  @StaplerFacet protected void legend() {}
  @StaplerFacet @StaplerPath("load-statistics") protected void
loadStatistics() {}
  @StaplerFacet protected void login() {}
  @StaplerFacet protected void loginError() {}
  @StaplerFacet protected void manag_() {}
  @StaplerFacet protected void manage() {}
  @StaplerFacet protected void newView() {}
  @StaplerFacet protected void noPrincipal() {}
  @StaplerFacet protected void oops() {}
  @StaplerFacet @StaplerPath("opensearch.xml") protected void
opensearchXml() {}
  @StaplerFacet @StaplerPath("projectRelationship-help") protected void
projectRelationshipHelp()
  @StaplerFacet protected void projectRelationship() {}
  @StaplerFacet protected void systemInfo() {}
  @StaplerFacet protected void threadDump() {}
  @StaplerFragment protected void _api() {}
  @StaplerFragment protected void downgrade() {}
  @StaplerFragment protected void sidepanel() {}
  ...
}

just to get the javadoc comments and leverage abstract and final... I
really do not like the extra effort of typing in all those method names...

Perhaps the biggest issue for me is that to retrofit that into Jenkins we
would need to take great care that these methods we are adding as facet and
fragment markers do not have names that may potentially conflict with
subclasses...

If we add

@StaplerObject
public abstract class Descriptor ... {
  ...
  @StaplerFragment protected abstract void config(); // doesn't really
matter if abstract or not here
  ...
}

we may make it impossible for a concrete subclass to have a

public MyDescriptor extends Descriptor ... {

  private MyConfig config;

  ...

  public MyConfig config() {
return config.clone();
  }

}

as they will be unable to retain binary compatibility 

Re: [Proposal] Making all things Stapler more declarative

[Kohsuke Kawaguchi]

FWIW, I've already discussed this offline with Stephen and I'm generally +1.

One suggestion I had for him is to consider using a Java method definition
instead of @StaplerFacet and @StaplerFragment. That is, instead of this:

@StaplerFacets({
  @StaplerFacet("index"),
  @StaplerFacet("config")
})
@StaplerFragments({
  @StaplerFragment("main"),
  @StaplerFragment("side-panel"),
  @StaplerFragment(value="tasks",optional=true)
})
public class Widget { ... }

Consider that:

public class Widget {
@StaplerFacet protected void index() {}
@StaplerFacet protected void config() {}

@StaplerFragment protected abstract void main();
@StaplerFragment("side-panel") protected abstract void sidepanel();
@StaplerFragment protected void tasks();
}

I liked this better because ...

   - It fits with the original design thinking behind Stapler, which is
   that views are like methods. Their inheritance and override semantics are
   all modeled after methods.
   - It avoids a long list of @StaplerFragment/@StaplerFacet at the top of
   the class declaration. We have some objects in the core that has quite a
   few views.
   - It creates a nice to place to describe view as javadoc.
   - 'abstract' and 'final' provides convenient semantics and javac does
   that work for us. One less thing to do for our annotation processor

Obvious downside is that those are not real methods.

On Mon, Jul 3, 2017 at 5:21 AM Stephen Connolly <
stephen.alan.conno...@gmail.com> wrote:

> I have been developing Jenkins plugins and changes to Jenkins core for
> more than 10 years now. As such, I have internalized a lot of the knowledge
> about how to develop against Jenkins and more specifically against Stapler.
>
> When I talk to people trying to start out development against Jenkins, the
> magic of Stapler seems to be a big source of confusion - at least to the
> people I have talked to.
>
> Prospective developers get confused:
>
>- between primary facets and facet fragments.
>- where to put facets and facet fragments.
>- which facet fragments are optional and which ones are mandatory
>
> (Did you even know that those jelly / groovy views are called facets and
> that there are two kinds?)
>
> Some of those concerns affect me too... but I am a seasoned Jenkins
> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
> facets and facet fragments from the class hierarchy and I can search each
> one looking for the  to
> discover that there is an optional facet fragment... and then try and dig
> through the Jelly / Groovy logic to determine when and why that fragment
> should be included.
>
> It doesn't just stop there... There are those methods that we expect
> Stapler to invoke for us... typically they are the doFillXYZItems() or
> doCheckXYZ() methods. I find myself having to mark them up like:
>
> @SuppressWarnings("unused") // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> So that my IDE stops complaining about the dead code. It's not a big deal
> in the grand scheme of things, but I'd much rather have an annotation on
> the method to signify that we expect the method to be used by stapler...
> Right now I could do that using
>
> @WebMethod(name="checkWidgetValue")
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> If I don't like that, I guess I could use the newer HTTP verb annotations
> in stapler:
>
> @GET // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> But the simple @GET is not screaming out stapler to me, so I feel
> compelled to add a // stapler comment to remind myself that the
> annotation is used by stapler
>
> Then we hit the actual name that stapler exposes things on. There are some
> conventions that stapler follows... and they are magic conventions... so
> much so that I just keep http://stapler.kohsuke.org/reference.html as the
> first bookmark in my bookmark bar.
>
> I think we can do better. I think we should do better, and here is my
> proposal.
>
> We should introduce some annotations. Make them available against older
> versions of Jenkins so that we don't have to wait for everyone to update
> their plugin baseline. The annotations can be added as a plugin dependency
> in the parent pom, that everyone can use them just by upgrading the plugin
> parent pom.
>
> Here are the class annotations I think we need:
>
>- An annotation (I propose @StaplerObject) that says "This object is
>expected to bound to a URL by Stapler, the Stapler annotations are
>complete, check that there are no facets / fragments without a
>corresponding annotation and no magic on this class please" (so if there
>is, say a rename.jelly but no @StaplerFacet("rename") or
>@StaplerFragment("rename") then you get an error)
>- An annotation (I propose @StaplerFacet and the repeatable container
>@StaplerFacets) that says 

Re: [Proposal] Making all things Stapler more declarative

[Kanstantsin Shautsou]

Not really, but imho almost all plugins had/has security issues with
unclear stapler endpoints and simple annotations for stapler method may be
better for automated security checks. I think it related for
stapler/binding understanding also.

2017-07-06 2:20 GMT+03:00 Jesse Glick :

> On Wed, Jul 5, 2017 at 6:50 PM, Kanstantsin Shautsou
>  wrote:
> > https://github.com/jenkinsci/github-plugin/blob/
> 93d40692ff3866705175624e93ec584d4ac88132/src/main/java/org/
> jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonit
> or.java#L182-L186
>
> These are not really related; they are just other interceptor
> annotations. (Which I do not really advocate, either—you should not
> use annotations where plain old method calls are just as readable, and
> far more flexible and debuggable.)
>
> --
> You received this message because you are subscribed to a topic in the
> Google Groups "Jenkins Developers" group.
> To unsubscribe from this topic, visit https://groups.google.com/d/
> topic/jenkinsci-dev/UrVVT8wbHIE/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> jenkinsci-dev+unsubscr...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/
> msgid/jenkinsci-dev/CANfRfr2ShkBiyLgxO18eK%2BKtecKMR5tKWbNfYHiOcc-
> HqgEG4g%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CAM9nkw8EbHV56idjwMr5w7scLOND5qH9HZLAkp7WsuBpVudBDA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Proposal] Making all things Stapler more declarative

[Oleg Nenashev]

Hi,

Generally +1 from me for Stapler annotations. It adds many extra 
opportunities:

   1. Self-documentation and Jenkins.io Documentation, e.g. via Annotation 
   Indexer
  - It will also allow to generate some Swagger specs for 
  Stapler-annotated API calls if somebody is ready to work on that
  2. Extra static analysis capabilities in default Parent POM
   - I have a draft of a Jenkins-specific detectors library 
  , but I put 
  it on hold to see how FindBugs/SpotBugs story finishes
   3. Ability to automatically generate some injected tests in JTH
   
Regarding the GSoC context, I would say that we had 95% of organizational 
issues and 5% of technical issues in the project. So I do not expect it to 
help it much, but OTOH new documentation would help with onboarding new 
developers.

So +1 from me

Best regards,
Oleg

четверг, 6 июля 2017 г., 0:50:59 UTC+2 пользователь Kanstantsin Shautsou 
написал:
>
>
>
> On Tuesday, July 4, 2017 at 8:25:06 PM UTC+3, Stephen Connolly wrote:
>>
>> Actually I nearly forgot Oleg you were driving the Google summer of code 
>> last year, what do you think?
>>
>>>
 And as usual seems you forgot github-plugin and Kirill that was GSOC 
> mentor for some UI changes and already covered with annotations some pieces 
> https://github.com/jenkinsci/github-plugin/blob/93d40692ff3866705175624e93ec584d4ac88132/src/main/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitor.java#L182-L186
>  
> 
>  
>  
>

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/a47a7728-a955-4ab5-b830-9dff98a3aa66%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Proposal] Making all things Stapler more declarative

[Jesse Glick]

On Wed, Jul 5, 2017 at 6:50 PM, Kanstantsin Shautsou
 wrote:
> https://github.com/jenkinsci/github-plugin/blob/93d40692ff3866705175624e93ec584d4ac88132/src/main/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitor.java#L182-L186

These are not really related; they are just other interceptor
annotations. (Which I do not really advocate, either—you should not
use annotations where plain old method calls are just as readable, and
far more flexible and debuggable.)

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/CANfRfr2ShkBiyLgxO18eK%2BKtecKMR5tKWbNfYHiOcc-HqgEG4g%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Proposal] Making all things Stapler more declarative

[Kanstantsin Shautsou]

On Tuesday, July 4, 2017 at 8:25:06 PM UTC+3, Stephen Connolly wrote:
>
> Actually I nearly forgot Oleg you were driving the Google summer of code 
> last year, what do you think?
>
>>
>>> And as usual seems you forgot github-plugin and Kirill that was GSOC 
mentor for some UI changes and already covered with annotations some pieces 
https://github.com/jenkinsci/github-plugin/blob/93d40692ff3866705175624e93ec584d4ac88132/src/main/java/org/jenkinsci/plugins/github/admin/GitHubHookRegisterProblemMonitor.java#L182-L186
 
 

-- 
You received this message because you are subscribed to the Google Groups 
"Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to jenkinsci-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/jenkinsci-dev/de7cee3b-6cf9-4c5e-8263-ee5395294657%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [Proposal] Making all things Stapler more declarative

[Ullrich Hafner]

> Am 05.07.2017 um 11:27 schrieb Stephen Connolly 
> :
> 
> +jenkins-dev

Uups, sorry…

> 
> On 5 July 2017 at 02:13, Ullrich Hafner  > wrote:
> 
>> Am 04.07.2017 um 18:44 schrieb Stephen Connolly 
>> >:
>> 
>> Ulli,
>> 
>> What do you think on this proposal, given that you have contact with a lot 
>> of students who have been trying to get to grips with the Jenkins code base.
>> 
>> Would it make their task easier?
>> 
> 
> Up to now most students work in the area of testing (ATH), so this will not 
> help them (they would love to see IDs for all entry fields, but that is a 
> totally different topic).
> 
> So one aspect where this could help is in knowing what to test.
> 
> The annotations can be used to help identify the intent of the plugin author 
> / core and we could perhaps even develop tooling to identify ATH coverage
> 
> 
>> -Stephen
>> 
>> On 3 July 2017 at 05:21, Stephen Connolly > > wrote:
>> I have been developing Jenkins plugins and changes to Jenkins core for more 
>> than 10 years now. As such, I have internalized a lot of the knowledge about 
>> how to develop against Jenkins and more specifically against Stapler.
>> 
>> When I talk to people trying to start out development against Jenkins, the 
>> magic of Stapler seems to be a big source of confusion - at least to the 
>> people I have talked to.
>> 
>> Prospective developers get confused:
>> 
>> between primary facets and facet fragments.
>> where to put facets and facet fragments.
>> which facet fragments are optional and which ones are mandatory
>> 
> 
> This indeed is one of the most confusing things when developing plug-ins. It 
> cost a lot of time to get these things right (mostly by copying and pasting 
> code from another plugin).
> 
> I'm not sure if annotations are the correct way of dealing with these 
> problems. I think you then need to know where to put an annotation and where 
> not, this will also result in searching for other plugins that do it right.
> 
> Yes, I would also propose improving the stapler documentation in conjunction 
> with these changes so that it should be easier to find the "recommended" 
> patterns to follow.
> 
> Though how much energy I personally have to push documentation improvements 
> is another question entirely ;-)

Well, this is always a problem ;-) But we already have some good examples in 
the wiki so the documentation of these new concepts might grow in the same way 
by some motivated plugin authors ...

> 
> 
> 
>> (Did you even know that those jelly / groovy views are called facets and 
>> that there are two kinds?)
>> 
>> Some of those concerns affect me too... but I am a seasoned Jenkins 
>> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of 
>> facets and facet fragments from the class hierarchy and I can search each 
>> one looking for the  to discover 
>> that there is an optional facet fragment... and then try and dig through the 
>> Jelly / Groovy logic to determine when and why that fragment should be 
>> included.
>> 
>> It doesn't just stop there... There are those methods that we expect Stapler 
>> to invoke for us... typically they are the doFillXYZItems() or doCheckXYZ() 
>> methods. I find myself having to mark them up like:
>> 
>> @SuppressWarnings("unused") // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>> 
>> So that my IDE stops complaining about the dead code. It's not a big deal in 
>> the grand scheme of things, but I'd much rather have an annotation on the 
>> method to signify that we expect the method to be used by stapler... Right 
>> now I could do that using
>> 
>> @WebMethod(name="checkWidgetValue")
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>> 
>> If I don't like that, I guess I could use the newer HTTP verb annotations in 
>> stapler:
>> 
>> @GET // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>> 
> 
> But do these annotations help to get rid of the unused code warnings? I think 
> you still need to add a @SuppressWarnings.
> 
> Well you can configure IDEs to say "this annotation is used" and we can even 
> have the Stapler plugin for IntelliJ pre-register the annotations so that 
> IntelliJ users (I'm selfish ;-) ) don't even need to add the setting

That would be very helpful! (Isn’t everybody using IntelliJ to develop 
plugins?;-)
Maybe improving the Stapler plug-in would be a good candidate for a 
bachelor/master thesis… (Code completion in jelly views would be awesome)

> 
> 
>> But the simple @GET is not screaming out stapler to me, so I feel compelled 
>> to add a // stapler comment to remind myself that the annotation is used 

Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

+jenkins-dev

On 5 July 2017 at 02:13, Ullrich Hafner  wrote:

>
> Am 04.07.2017 um 18:44 schrieb Stephen Connolly <
> stephen.alan.conno...@gmail.com>:
>
> Ulli,
>
> What do you think on this proposal, given that you have contact with a lot
> of students who have been trying to get to grips with the Jenkins code base.
>
> Would it make their task easier?
>
>
> Up to now most students work in the area of testing (ATH), so this will
> not help them (they would love to see IDs for all entry fields, but that is
> a totally different topic).
>

So one aspect where this could help is in knowing what to test.

The annotations can be used to help identify the intent of the plugin
author / core and we could perhaps even develop tooling to identify ATH
coverage


>
> -Stephen
>
> On 3 July 2017 at 05:21, Stephen Connolly  > wrote:
>
>> I have been developing Jenkins plugins and changes to Jenkins core for
>> more than 10 years now. As such, I have internalized a lot of the knowledge
>> about how to develop against Jenkins and more specifically against Stapler.
>>
>> When I talk to people trying to start out development against Jenkins,
>> the magic of Stapler seems to be a big source of confusion - at least to
>> the people I have talked to.
>>
>> Prospective developers get confused:
>>
>>- between primary facets and facet fragments.
>>- where to put facets and facet fragments.
>>- which facet fragments are optional and which ones are mandatory
>>
>>
>>
> This indeed is one of the most confusing things when developing plug-ins.
> It cost a lot of time to get these things right (mostly by copying and
> pasting code from another plugin).
>
> I'm not sure if annotations are the correct way of dealing with these
> problems. I think you then need to know where to put an annotation and
> where not, this will also result in searching for other plugins that do it
> right.
>

Yes, I would also propose improving the stapler documentation in
conjunction with these changes so that it should be easier to find the
"recommended" patterns to follow.

Though how much energy I personally have to push documentation improvements
is another question entirely ;-)


>
>
> (Did you even know that those jelly / groovy views are called facets and
>> that there are two kinds?)
>>
>> Some of those concerns affect me too... but I am a seasoned Jenkins
>> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
>> facets and facet fragments from the class hierarchy and I can search each
>> one looking for the  to
>> discover that there is an optional facet fragment... and then try and dig
>> through the Jelly / Groovy logic to determine when and why that fragment
>> should be included.
>>
>> It doesn't just stop there... There are those methods that we expect
>> Stapler to invoke for us... typically they are the doFillXYZItems() or
>> doCheckXYZ() methods. I find myself having to mark them up like:
>>
>> @SuppressWarnings("unused") // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>> So that my IDE stops complaining about the dead code. It's not a big deal
>> in the grand scheme of things, but I'd much rather have an annotation on
>> the method to signify that we expect the method to be used by stapler...
>> Right now I could do that using
>>
>> @WebMethod(name="checkWidgetValue")
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>> If I don't like that, I guess I could use the newer HTTP verb annotations
>> in stapler:
>>
>> @GET // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>>
> But do these annotations help to get rid of the unused code warnings? I
> think you still need to add a @SuppressWarnings.
>

Well you can configure IDEs to say "this annotation is used" and we can
even have the Stapler plugin for IntelliJ pre-register the annotations so
that IntelliJ users (I'm selfish ;-) ) don't even need to add the setting


>
> But the simple @GET is not screaming out stapler to me, so I feel
>> compelled to add a // stapler comment to remind myself that the
>> annotation is used by stapler
>>
>> Then we hit the actual name that stapler exposes things on. There are
>> some conventions that stapler follows... and they are magic conventions...
>> so much so that I just keep http://stapler.kohsuke.org/reference.html as
>> the first bookmark in my bookmark bar.
>>
>> I think we can do better. I think we should do better, and here is my
>> proposal.
>>
>> We should introduce some annotations. Make them available against older
>> versions of Jenkins so that we don't have to wait for everyone to update
>> their plugin baseline. The annotations can be added as a plugin dependency
>> in the parent pom, that everyone can use them just by upgrading the plugin
>> parent pom.
>>
>> Here are the class 

Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

Actually I nearly forgot Oleg you were driving the Google summer of code
last year, what do you think?

On Tue 4 Jul 2017 at 17:44, Stephen Connolly <
stephen.alan.conno...@gmail.com> wrote:

> Ulli,
>
> What do you think on this proposal, given that you have contact with a lot
> of students who have been trying to get to grips with the Jenkins code base.
>
> Would it make their task easier?
>
> -Stephen
>
> On 3 July 2017 at 05:21, Stephen Connolly  > wrote:
>
>> I have been developing Jenkins plugins and changes to Jenkins core for
>> more than 10 years now. As such, I have internalized a lot of the knowledge
>> about how to develop against Jenkins and more specifically against Stapler.
>>
>> When I talk to people trying to start out development against Jenkins,
>> the magic of Stapler seems to be a big source of confusion - at least to
>> the people I have talked to.
>>
>> Prospective developers get confused:
>>
>>- between primary facets and facet fragments.
>>- where to put facets and facet fragments.
>>- which facet fragments are optional and which ones are mandatory
>>
>> (Did you even know that those jelly / groovy views are called facets and
>> that there are two kinds?)
>>
>> Some of those concerns affect me too... but I am a seasoned Jenkins
>> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
>> facets and facet fragments from the class hierarchy and I can search each
>> one looking for the  to
>> discover that there is an optional facet fragment... and then try and dig
>> through the Jelly / Groovy logic to determine when and why that fragment
>> should be included.
>>
>> It doesn't just stop there... There are those methods that we expect
>> Stapler to invoke for us... typically they are the doFillXYZItems() or
>> doCheckXYZ() methods. I find myself having to mark them up like:
>>
>> @SuppressWarnings("unused") // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>> So that my IDE stops complaining about the dead code. It's not a big deal
>> in the grand scheme of things, but I'd much rather have an annotation on
>> the method to signify that we expect the method to be used by stapler...
>> Right now I could do that using
>>
>> @WebMethod(name="checkWidgetValue")
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>> If I don't like that, I guess I could use the newer HTTP verb annotations
>> in stapler:
>>
>> @GET // stapler
>> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
>> ...
>> }
>>
>>
>> But the simple @GET is not screaming out stapler to me, so I feel
>> compelled to add a // stapler comment to remind myself that the
>> annotation is used by stapler
>>
>> Then we hit the actual name that stapler exposes things on. There are
>> some conventions that stapler follows... and they are magic conventions...
>> so much so that I just keep http://stapler.kohsuke.org/reference.html as
>> the first bookmark in my bookmark bar.
>>
>> I think we can do better. I think we should do better, and here is my
>> proposal.
>>
>> We should introduce some annotations. Make them available against older
>> versions of Jenkins so that we don't have to wait for everyone to update
>> their plugin baseline. The annotations can be added as a plugin dependency
>> in the parent pom, that everyone can use them just by upgrading the plugin
>> parent pom.
>>
>> Here are the class annotations I think we need:
>>
>>- An annotation (I propose @StaplerObject) that says "This object is
>>expected to bound to a URL by Stapler, the Stapler annotations are
>>complete, check that there are no facets / fragments without a
>>corresponding annotation and no magic on this class please" (so if there
>>is, say a rename.jelly but no @StaplerFacet("rename") or
>>@StaplerFragment("rename") then you get an error)
>>- An annotation (I propose @StaplerFacet and the repeatable container
>>@StaplerFacets) that says "Here are the facets that this object is
>>expected to have" (doesn't have to be on a @StaplerObject but when
>>not on a @StaplerObject we can only check for missing expected
>>facets, we cannot check for "unexpected" facets - i.e. I created a facet
>>with a spelling mistake or typo in the name)
>>- An annotation (I propose @StaplerFragment and the repeatable
>>container @StaplerFragments) that says "Here are the facet fragments
>>that this object is required or may optionally have" (quite often will not
>>be on a @StaplerObject)
>>
>>
>> So I envision something like this (pre-Java 8):
>>
>> @StaplerObject
>> @StaplerFacets({
>>   @StaplerFacet("index"),
>>   @StaplerFacet("config")
>> })
>> @StaplerFragments({
>>   @StaplerFragment("main"),
>>   @StaplerFragment("side-panel"),
>>   @StaplerFragment(value="tasks",optional=true)
>> })
>> public class Widget { ... }
>>

Re: [Proposal] Making all things Stapler more declarative

[Stephen Connolly]

Ulli,

What do you think on this proposal, given that you have contact with a lot
of students who have been trying to get to grips with the Jenkins code base.

Would it make their task easier?

-Stephen

On 3 July 2017 at 05:21, Stephen Connolly 
wrote:

> I have been developing Jenkins plugins and changes to Jenkins core for
> more than 10 years now. As such, I have internalized a lot of the knowledge
> about how to develop against Jenkins and more specifically against Stapler.
>
> When I talk to people trying to start out development against Jenkins, the
> magic of Stapler seems to be a big source of confusion - at least to the
> people I have talked to.
>
> Prospective developers get confused:
>
>- between primary facets and facet fragments.
>- where to put facets and facet fragments.
>- which facet fragments are optional and which ones are mandatory
>
> (Did you even know that those jelly / groovy views are called facets and
> that there are two kinds?)
>
> Some of those concerns affect me too... but I am a seasoned Jenkins
> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
> facets and facet fragments from the class hierarchy and I can search each
> one looking for the  to
> discover that there is an optional facet fragment... and then try and dig
> through the Jelly / Groovy logic to determine when and why that fragment
> should be included.
>
> It doesn't just stop there... There are those methods that we expect
> Stapler to invoke for us... typically they are the doFillXYZItems() or
> doCheckXYZ() methods. I find myself having to mark them up like:
>
> @SuppressWarnings("unused") // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> So that my IDE stops complaining about the dead code. It's not a big deal
> in the grand scheme of things, but I'd much rather have an annotation on
> the method to signify that we expect the method to be used by stapler...
> Right now I could do that using
>
> @WebMethod(name="checkWidgetValue")
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> If I don't like that, I guess I could use the newer HTTP verb annotations
> in stapler:
>
> @GET // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> But the simple @GET is not screaming out stapler to me, so I feel
> compelled to add a // stapler comment to remind myself that the
> annotation is used by stapler
>
> Then we hit the actual name that stapler exposes things on. There are some
> conventions that stapler follows... and they are magic conventions... so
> much so that I just keep http://stapler.kohsuke.org/reference.html as the
> first bookmark in my bookmark bar.
>
> I think we can do better. I think we should do better, and here is my
> proposal.
>
> We should introduce some annotations. Make them available against older
> versions of Jenkins so that we don't have to wait for everyone to update
> their plugin baseline. The annotations can be added as a plugin dependency
> in the parent pom, that everyone can use them just by upgrading the plugin
> parent pom.
>
> Here are the class annotations I think we need:
>
>- An annotation (I propose @StaplerObject) that says "This object is
>expected to bound to a URL by Stapler, the Stapler annotations are
>complete, check that there are no facets / fragments without a
>corresponding annotation and no magic on this class please" (so if there
>is, say a rename.jelly but no @StaplerFacet("rename") or
>@StaplerFragment("rename") then you get an error)
>- An annotation (I propose @StaplerFacet and the repeatable container
>@StaplerFacets) that says "Here are the facets that this object is
>expected to have" (doesn't have to be on a @StaplerObject but when not
>on a @StaplerObject we can only check for missing expected facets, we
>cannot check for "unexpected" facets - i.e. I created a facet with a
>spelling mistake or typo in the name)
>- An annotation (I propose @StaplerFragment and the repeatable
>container @StaplerFragments) that says "Here are the facet fragments
>that this object is required or may optionally have" (quite often will not
>be on a @StaplerObject)
>
>
> So I envision something like this (pre-Java 8):
>
> @StaplerObject
> @StaplerFacets({
>   @StaplerFacet("index"),
>   @StaplerFacet("config")
> })
> @StaplerFragments({
>   @StaplerFragment("main"),
>   @StaplerFragment("side-panel"),
>   @StaplerFragment(value="tasks",optional=true)
> })
> public class Widget { ... }
>
>
> When compiled targeting Java 8 this would become:
>
> @StaplerObject
> @StaplerFacet("index")
> @StaplerFacet("config")
> @StaplerFragment("main")
> @StaplerFragment("side-panel")
> @StaplerFragment(value="tasks",optional=true)
> public class Widget { ... }
>
>
> There would be an annotation processor that could 

Re: [Proposal] Making all things Stapler more declarative

[Robert Sandell]

> Would this change have helped you get started quicker?
Yes, If I can remember that far back I believe it would have helped, 90% of
learning how to code plugins is looking at what others have done before.

> Would this change help you even now?
Yes, even though my brain usually reads "stapler method" on any doXXX
method having them "marked up" would help more with code reviews and while
planning an implementation and especially refactoring.

> How ugly are my proposed annotation names? Can you provide a less ugly
scheme of names?
I think they are OK, can't come up with anything better :)

> Anything else?
So with this we could perhaps enhance the existing tooling even more, like
some intelligent inspections in the IDEA plugin and perhaps even a "go to
facet" and "create facet" :)

/B

2017-07-03 14:21 GMT+02:00 Stephen Connolly :

> I have been developing Jenkins plugins and changes to Jenkins core for
> more than 10 years now. As such, I have internalized a lot of the knowledge
> about how to develop against Jenkins and more specifically against Stapler.
>
> When I talk to people trying to start out development against Jenkins, the
> magic of Stapler seems to be a big source of confusion - at least to the
> people I have talked to.
>
> Prospective developers get confused:
>
>- between primary facets and facet fragments.
>- where to put facets and facet fragments.
>- which facet fragments are optional and which ones are mandatory
>
> (Did you even know that those jelly / groovy views are called facets and
> that there are two kinds?)
>
> Some of those concerns affect me too... but I am a seasoned Jenkins
> developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
> facets and facet fragments from the class hierarchy and I can search each
> one looking for the  to
> discover that there is an optional facet fragment... and then try and dig
> through the Jelly / Groovy logic to determine when and why that fragment
> should be included.
>
> It doesn't just stop there... There are those methods that we expect
> Stapler to invoke for us... typically they are the doFillXYZItems() or
> doCheckXYZ() methods. I find myself having to mark them up like:
>
> @SuppressWarnings("unused") // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> So that my IDE stops complaining about the dead code. It's not a big deal
> in the grand scheme of things, but I'd much rather have an annotation on
> the method to signify that we expect the method to be used by stapler...
> Right now I could do that using
>
> @WebMethod(name="checkWidgetValue")
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> If I don't like that, I guess I could use the newer HTTP verb annotations
> in stapler:
>
> @GET // stapler
> public FormValidation doCheckWidgetValue(@QueryParameter String value) {
> ...
> }
>
>
> But the simple @GET is not screaming out stapler to me, so I feel
> compelled to add a // stapler comment to remind myself that the
> annotation is used by stapler
>
> Then we hit the actual name that stapler exposes things on. There are some
> conventions that stapler follows... and they are magic conventions... so
> much so that I just keep http://stapler.kohsuke.org/reference.html as the
> first bookmark in my bookmark bar.
>
> I think we can do better. I think we should do better, and here is my
> proposal.
>
> We should introduce some annotations. Make them available against older
> versions of Jenkins so that we don't have to wait for everyone to update
> their plugin baseline. The annotations can be added as a plugin dependency
> in the parent pom, that everyone can use them just by upgrading the plugin
> parent pom.
>
> Here are the class annotations I think we need:
>
>- An annotation (I propose @StaplerObject) that says "This object is
>expected to bound to a URL by Stapler, the Stapler annotations are
>complete, check that there are no facets / fragments without a
>corresponding annotation and no magic on this class please" (so if there
>is, say a rename.jelly but no @StaplerFacet("rename") or
>@StaplerFragment("rename") then you get an error)
>- An annotation (I propose @StaplerFacet and the repeatable container
>@StaplerFacets) that says "Here are the facets that this object is
>expected to have" (doesn't have to be on a @StaplerObject but when not
>on a @StaplerObject we can only check for missing expected facets, we
>cannot check for "unexpected" facets - i.e. I created a facet with a
>spelling mistake or typo in the name)
>- An annotation (I propose @StaplerFragment and the repeatable
>container @StaplerFragments) that says "Here are the facet fragments
>that this object is required or may optionally have" (quite often will not
>be on a @StaplerObject)
>
>
> So I envision something like this (pre-Java 8):
>

[Proposal] Making all things Stapler more declarative

[Stephen Connolly]

I have been developing Jenkins plugins and changes to Jenkins core for more
than 10 years now. As such, I have internalized a lot of the knowledge
about how to develop against Jenkins and more specifically against Stapler.

When I talk to people trying to start out development against Jenkins, the
magic of Stapler seems to be a big source of confusion - at least to the
people I have talked to.

Prospective developers get confused:

   - between primary facets and facet fragments.
   - where to put facets and facet fragments.
   - which facet fragments are optional and which ones are mandatory

(Did you even know that those jelly / groovy views are called facets and
that there are two kinds?)

Some of those concerns affect me too... but I am a seasoned Jenkins
developer CMD+SHIFT+P, *, ENTER and IntelliJ is showing me a full list of
facets and facet fragments from the class hierarchy and I can search each
one looking for the  to
discover that there is an optional facet fragment... and then try and dig
through the Jelly / Groovy logic to determine when and why that fragment
should be included.

It doesn't just stop there... There are those methods that we expect
Stapler to invoke for us... typically they are the doFillXYZItems() or
doCheckXYZ() methods. I find myself having to mark them up like:

@SuppressWarnings("unused") // stapler
public FormValidation doCheckWidgetValue(@QueryParameter String value) {
...
}


So that my IDE stops complaining about the dead code. It's not a big deal
in the grand scheme of things, but I'd much rather have an annotation on
the method to signify that we expect the method to be used by stapler...
Right now I could do that using

@WebMethod(name="checkWidgetValue")
public FormValidation doCheckWidgetValue(@QueryParameter String value) {
...
}


If I don't like that, I guess I could use the newer HTTP verb annotations
in stapler:

@GET // stapler
public FormValidation doCheckWidgetValue(@QueryParameter String value) {
...
}


But the simple @GET is not screaming out stapler to me, so I feel compelled
to add a // stapler comment to remind myself that the annotation is used by
stapler

Then we hit the actual name that stapler exposes things on. There are some
conventions that stapler follows... and they are magic conventions... so
much so that I just keep http://stapler.kohsuke.org/reference.html as the
first bookmark in my bookmark bar.

I think we can do better. I think we should do better, and here is my
proposal.

We should introduce some annotations. Make them available against older
versions of Jenkins so that we don't have to wait for everyone to update
their plugin baseline. The annotations can be added as a plugin dependency
in the parent pom, that everyone can use them just by upgrading the plugin
parent pom.

Here are the class annotations I think we need:

   - An annotation (I propose @StaplerObject) that says "This object is
   expected to bound to a URL by Stapler, the Stapler annotations are
   complete, check that there are no facets / fragments without a
   corresponding annotation and no magic on this class please" (so if there
   is, say a rename.jelly but no @StaplerFacet("rename") or
   @StaplerFragment("rename") then you get an error)
   - An annotation (I propose @StaplerFacet and the repeatable container
   @StaplerFacets) that says "Here are the facets that this object is
   expected to have" (doesn't have to be on a @StaplerObject but when not
   on a @StaplerObject we can only check for missing expected facets, we
   cannot check for "unexpected" facets - i.e. I created a facet with a
   spelling mistake or typo in the name)
   - An annotation (I propose @StaplerFragment and the repeatable container
   @StaplerFragments) that says "Here are the facet fragments that this
   object is required or may optionally have" (quite often will not be on a
   @StaplerObject)


So I envision something like this (pre-Java 8):

@StaplerObject
@StaplerFacets({
  @StaplerFacet("index"),
  @StaplerFacet("config")
})
@StaplerFragments({
  @StaplerFragment("main"),
  @StaplerFragment("side-panel"),
  @StaplerFragment(value="tasks",optional=true)
})
public class Widget { ... }


When compiled targeting Java 8 this would become:

@StaplerObject
@StaplerFacet("index")
@StaplerFacet("config")
@StaplerFragment("main")
@StaplerFragment("side-panel")
@StaplerFragment(value="tasks",optional=true)
public class Widget { ... }


There would be an annotation processor that could do some checks:

   - If you add @StaplerFragment(..., optional=false) then any non-abstract
   class must have that fragment somewhere in the class hierarchy
   - If you add @StaplerFacet then any non-abstract class must have that
   fragment somewhere in the class hierarchy
   - If you add @StaplerObject then only the named facets and facet
   fragments must be present for that class (to catch somebody calling the
   facet resource with an incorrect name - it should be side-panel2.jelly
   but