-
-
Notifications
You must be signed in to change notification settings - Fork 272
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FindBugs - Reconsider filters and cleanup some issues #96
Conversation
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
@@ -155,12 +156,6 @@ THE SOFTWARE. | |||
<optional>true</optional><!-- no need to have this at runtime --> | |||
</dependency> | |||
<dependency> | |||
<groupId>com.github.stephenc.findbugs</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@stephenc I think it's no longer required, right? Or is it a licensing issue again?
Ready to go, hehe |
CC @olivergondza , who has fixed a couple of issues in his PR |
…EFER_ZERO_LENGTH_ARRAYS)
…UALS_SHOULD_HANDLE_NULL_ARGUMENT)
…MAT_STRING_USES_NEWLINE)
…of wrong inheritance
…JBoss6 (SBSC_USE_STRINGBUFFER_CONCATENATION)
…m another project
…IC_INNER_SHOULD_BE_STATIC)
Only 70 issues left... |
…appens only on race condition)
@@ -81,7 +81,7 @@ protected URL retrieve(Channel channel, long sum1, long sum2) throws IOException | |||
Checksum actual = Checksum.forFile(tmp); | |||
if (!expected.equals(actual)) { | |||
throw new IOException(String.format( | |||
"Incorrect checksum of retrieved jar: %s\nExpected: %s\nActual: %s", | |||
"Incorrect checksum of retrieved jar: %s%nExpected: %s\nActual: %s", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The other instance on the same lines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, missed them
@@ -177,9 +180,9 @@ private Channel channel() { | |||
* @throws IOException if the channel is disconnected or otherwise unavailable. | |||
* @since FIXME after merge |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe clean up these while you are at it…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too many TODOs, gonna to follow-up in a separate PR
🐝 |
@reviewbybees done |
This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging. |
@reviewbybees done |
Changes are listed below: Fixed issues: * [JENKINS-22853](https://issues.jenkins-ci.org/browse/JENKINS-22853) - Be robust against the delayed EOF command when unexporting input and output streams. (jenkinsci/remoting#97) * Fixed ~20 minor issues reported by FindBugs. More fixes to be delivered in future versions. (jenkinsci/remoting#96) Enhancements: * [JENKINS-37218](https://issues.jenkins-ci.org/browse/JENKINS-37218) - Performance: <code>ClassFilter</code> does not use Regular Expressions anymore to match <code>String.startsWith</code> patterns. (jenkinsci/remoting#92) * [JENKINS-37031](https://issues.jenkins-ci.org/browse/JENKINS-37031) <code>TcpSlaveAgentListener</code> now publishes a list of supported agent protocols to speed up connection setup. (jenkinsci/remoting#93)
(Cherry Pick of 06be933) Changes are listed below: Fixed issues: * [JENKINS-22853](https://issues.jenkins-ci.org/browse/JENKINS-22853) - Be robust against the delayed EOF command when unexporting input and output streams. (jenkinsci/remoting#97) * Fixed ~20 minor issues reported by FindBugs. More fixes to be delivered in future versions. (jenkinsci/remoting#96) Enhancements: * [JENKINS-37218](https://issues.jenkins-ci.org/browse/JENKINS-37218) - Performance: <code>ClassFilter</code> does not use Regular Expressions anymore to match <code>String.startsWith</code> patterns. (jenkinsci/remoting#92) * [JENKINS-37031](https://issues.jenkins-ci.org/browse/JENKINS-37031) <code>TcpSlaveAgentListener</code> now publishes a list of supported agent protocols to speed up connection setup. (jenkinsci/remoting#93)
I would like to restore FindBugs checks in the code. Current filters hide away much real issues, and I would rather admit issues and try to fix it instead of hiding my head in sand.
@reviewbybees