-
-
Notifications
You must be signed in to change notification settings - Fork 9k
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
[JENKINS-36720] - Cleanup of SOME medium FindBugs issues #2519
[JENKINS-36720] - Cleanup of SOME medium FindBugs issues #2519
Conversation
…how receive empty response
…aplerRequest in nonnull with few exceptions
@@ -188,6 +188,9 @@ private Channel connectViaCliPort(URL jenkins, CliPort clip) throws IOException | |||
rsp.write(ch); | |||
} | |||
String head = new BufferedReader(new StringReader(rsp.toString("ISO-8859-1"))).readLine(); | |||
if (head == null) { | |||
throw new IOException("Unexpected empty response"); |
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.
Clarify from what. According to comment before Proxy response body must be not empty
@jenkinsci/code-reviewers Ready to go? |
@@ -77,6 +77,7 @@ | |||
import java.util.IdentityHashMap; | |||
import javax.annotation.CheckForNull; | |||
import javax.annotation.Nonnull; | |||
import javax.annotation.Nullable; |
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.
@oleg-nenashev Should we be using a FindBugs annotation here as per @stephenc's licensing comments?
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 committed to do some bulk rework on this front, but he has not followed up yet. By now all FB annotations annotations are marked as deprecated, and I do not consider using them. IANAL, and my objective is to fix the code
@daniel-beck WDYT about merging it? |
@KostyaSha are you fine with my feedback? |
Seems fine. |
Merging just to get the change integrated. |
Started from 480 issues, after the current changes there are only 417 left...
Obviously, I'm not going to fix all of them in short-term.