[PINT-287] Changed plugin to Java 11

jenkinsci / datadog-plugin

A Jenkins plugin used to forward metrics, events, and service checks to an account at Datadog, automatically.

https://plugins.jenkins.io/datadog/

MIT License

30 stars 48 forks source link

[PINT-287] Changed plugin to Java 11 #404

Closed rahulkaukuntla closed 3 months ago

rahulkaukuntla commented 4 months ago

What does this PR do?

Updating plugin to Java 11.

Description of the Change

Alternate Designs

Possible Drawbacks

Verification Process

Additional Notes

Release Notes

Review checklist (to be filled by reviewers)

[ ] Feature or bug fix MUST have appropriate tests (unit, integration, etc...)
[ ] PR title must be written as a CHANGELOG entry (see why)
[ ] Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
[ ] PR must have one changelog/ label attached. If applicable it should have the backward-incompatible label attached.
[ ] PR should not have do-not-merge/ label attached.
[ ] If Applicable, issue must have kind/ and severity/ labels attached at least.

nikita-tkachenko-datadog commented 3 months ago

To be honest, I do not quite follow the "NPEs are not allowed to be caught anymore" argument. What is it that makes catching NPEs impossible? If it is the Spotbugs plugin, then there's an annotation one can use to suppress Spotbugs warnings.

This is not to say catching NPEs is a good practice. The problem here is that the code is rather old and it is difficult to reason about these catches and the problems they were supposedly fixing. From my perspective the safest bet would be to leave them intact.

rahulkaukuntla commented 3 months ago

To be honest, I do not quite follow the "NPEs are not allowed to be caught anymore" argument. What is it that makes catching NPEs impossible? If it is the Spotbugs plugin, then there's an annotation one can use to suppress Spotbugs warnings.

This is not to say catching NPEs is a good practice. The problem here is that the code is rather old and it is difficult to reason about these catches and the problems they were supposedly fixing. From my perspective the safest bet would be to leave them intact.

@nikita-tkachenko-datadog, thank you for this information, I wasn't aware that the warnings could just be suppressed. In the case of the getPauseDurationMillis method in DatadogGraphListener.java, trying to avoid catching NPEs seems to require a large and unneeded refactor. In BuildData.java as well, I wasn't fully sure if an NPE was impossible to catch. As such, I have done your suggestion, and just suppressed the warning.