Maintain lock ordering when reporting in environment variables

[gaspardpetit]

Fixes #300 : The order of the locked resources in the environment variable does not match the order in the extra parameter

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
• Ensure that the pull request title represents the desired changelog entry
• Please describe what you did
• Link to relevant issues in GitHub or Jira
• Link to relevant pull requests, esp. upstream and downstream changes
• Ensure you have provided tests - that demonstrates feature works or fixes the issue

[jimklimov]

Nice catch, so a set vs. list, huh?

[gaspardpetit]

yes - but after this is merged in, I will refactor this a bit. For now we just pass around a list (or set) of lock names, but with #299 we start having to pass around the properties as well. I'd like to allow each extra to have its own variable, and we'll now have to pass around the variable as well. So after this and #299 are merged, I'll wrap all of this in a list of lock instance structures, including description which is also passed around but does not always correspond to the selected lock when extra have been used... so to be continued....

[basil]

Released in 2.15.

[joshsleeper]

so I'm by no means a java person, but I am a dev who uses Jenkins.
can someone explain to me why this param type change from Set to List isn't considered a breaking change, or even worth a callout in the release notes?

it breaks groovy jobs like mine that call lock() directly because it was previously being created as a Set to match the API.

is this a release notes/documentation failure, or am I misusing the plugin maybe?

[gaspardpetit]

Hi @joshsleeper

Definitely a failure to report in the release notes. While I am an experienced Java developer, I only just recently started contributing to Jenkins plugins. My experience with them is to only use operations exposed as steps, and I initially did not realize some people were calling internal plug-in methods directly and had backward compatibility expectations on them. I'll definitely be more careful to flag these breaking changes going forward and for now I can only apologize.

Gaspard

[joshsleeper]

no anger here and plenty of understanding~
this isn't the first upgrade of this plugin that's busted things for us, so figured we were better off calling it out in the hopes that it doesn't happen every time!

I would suggest, if possible, to consider updating the release notes to mention the breaking change~
after all, changing the calling pattern of anything that's public is a breaking change! 😆

that aside, jenkins plugin maintenance def isn't a job folks do for glory so thanks for all your hard work!