Code improvements

[offa]

• Code clean-up
• Deprecated APIs replaced (still some left)
• Missing nullability annotations added
• JSR 305 annotations replaced

---

• 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

[jglick]

Ran out of time to review further. Will require detailed review from someone experienced in Jenkins code before merging to ensure there are no regressions.

[offa]

Thanks for your review, I have addressed all comments.

[Wadeck]

👍 For Lambda, NonNull refactoring, import optimization it seems to be fine. (those are proposed by IntelliJ)

⁉️ But for getDeclaredConstructor, I will not merge this without an explanation of the benefit using this approach, it seems less interesting than current version. Perhaps it's just me not knowing why you are doing that ;-)
So, either you provide the advantage/rationale of that approach or you remove them from this PR so that I can merge it.

💡 In general, I would say that I prefer to have PRs without a mix of manual proposals and IntelliJ (or other tool) proposals. The manual ones require more review than the automated ones. And saying it came from a tool in the PR description also helps ;)

Thanks for your contribution @offa 👍

[Wadeck]

If you want me to update your PR and merge it just tell me ;)
(without constructor changes, with the NonNull annotation on WrappedCredentialsStore changed and the getById changed)

[offa]

⁉️ But for getDeclaredConstructor, I will not merge this without an explanation of the benefit using this approach, it seems less interesting than current version. Perhaps it's just me not knowing why you are doing that ;-)
So, either you provide the advantage/rationale of that approach or you remove them from this PR so that I can merge it.

I'm on Java 11+ and newInstance() is deprecated since Java 9 (Java 11 docs). The docs (and discussion on SO) suggest getDeclaredConstructor().newInstance() as the replacement.

However, it's better to get a consensus here, so I'll remove it from this PR and in doubt provide a follow up.

[offa]

If you want me to update your PR and merge it just tell me ;)
(without constructor changes, with the NonNull annotation on WrappedCredentialsStore changed and the getById changed)

Thanks 😄, but I've committed getById() through PR suggestion, newInstance() is reverted (I'll push once tested locally); regarding annotation, please see my comment above.

[Wadeck]

However, it's better to get a consensus here, so I'll remove it from this PR and in doubt provide a follow up.

Perfect yes thank you :)

regarding annotation, please see my comment above.

Will check the latest version and adjust if necessary before merge 👍

I'm on Java 11+ [...]

Thanks for the link and the rationale, will keep that in a place of my brain for next reviews / plugins 👍

[Wadeck]

Will merge once the CI is good :)

[Wadeck]

@offa Thanks for the contribution! Don't hesitate to make others, Hacktoberfest started today, you can gain a t-shirt with 5 approved PRs during October, more info on https://hacktoberfest.digitalocean.com/

[jglick]

Use getConstructor(), not getDeclaredConstructor(), since in the contexts in which we would be calling newInstance() we expect the no-arg constructor to be public.

[jglick]

(still reviewing)

[jglick]

Looks good other than a couple of nits. Thanks!