Updated documentation for JNLP slaves for newer Jenkins versions

[MadsNielsen]

Found out while creating a demo, that recent changes in remoting requires a new configuration for JNLP slaves.

So i updated the documentation with the new tidbits.

[oleg-nenashev]

Maybe just drop the support to KISS?

[MadsNielsen]

That is a fair point, do we have a policy about this as at all?
I'd prefer to keep it for now, until we jot down and agree on a backwards compatibility strategy, and agree on updating the Jenkins version in the pom at regular intervals.
One thing i'm willing to do is to move the newest configuration to the top, and mark the old configuration deprecated so that the first thing you see is the most recent configuration.

[jglick]

do we have a policy about this

@kohsuke has not formally said anything, and if you are maintaining the plugin you set the policy, but read this if you have not already.

[jglick]

As in #168, I would hope this is optional.

[jglick]

do we have a policy about this

@kohsuke has not formally said anything, and if you are maintaining the plugin you set the policy, but read this if you have not already.

[oleg-nenashev]

The plugin needs workDir settings as a mandatory dependency, because the field is marked as Nonnull here: https://github.com/jenkinsci/jenkins/blob/393680beb5d64628bdf8d9e5bf8daf4cab2191ad/core/src/main/java/hudson/slaves/JNLPLauncher.java#L69-L77 .

There is a default constructor which does not require working directory, but IIUC this plugin is unable to discover that constructor.

It may be a serious compatibility issue for the CasC design - many plugins evolve by creating new constructors and deprecating existing ones. In such case the user's configuration YAML will be broken during update.

@MadsNielsen @ewelinawilkosz @ndeloof My proposal would be to see whether it is possible to update the plugin to heck for deprecated public constructor in order to retain compatibility in such case

[ndeloof]

config-as-code assumes databound constructor do cleanly reflect the component requirements.
by design, update to core|plugins do change the data model and as such can require updates to yaml file. There's no such thing like a yaml backward compatibility expected here.

[oleg-nenashev]

OK 🤷‍♂️

[oleg-nenashev]

Obviously, we could modify Jenkins Core in this particular case.

[ndeloof]

I also expect plugin developer could take more care not changing DataBoundConstructor but for newly introduced required parameters. They can rely on DataBoundSetters to introduce optional parameters. AFAICT changing constructor has been used a lot in the past because we missed this option.

[oleg-nenashev]

@DataBoundSetter still has a slight design flaw. Whatever you move to setter, becomes a part of public API, and you cannot make the field final. So one either has to handle non-constructor modifications or somehow restrict the field

[ndeloof]

Whatever you move to setter, becomes a part of public API

Just like a public constructor parameter and associated getter for databinding, right ?

and you cannot make the field final

As long as you don't explose it as a mutable data structure on getter, that shouldn't be an issue. Same issue applies in many places with data migration from readResolve with some /* mostly final */ comment in code.

[ewelinawilkosz]

just to make sure - cause there was a long discussion in this PR - is it ok to merge that one?

[oleg-nenashev]

IMHO we need #234 instead, but I am fine with the merge if it helps to move forward

[ewelinawilkosz]

I'm just trying to clean up, haven't got to #234 yet

[MadsNielsen]

I'm fine with not merging this, if we can merge #234. I like @oleg-nenashev elegant solution. It's glue code for the first period, until people start writing proper plugins with correct use of @DataBound.

[oleg-nenashev]

@MadsNielsen It can be closed once #234 is released

[ewelinawilkosz]

#234 merged, closing this one as decided above