-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix new error found in findbugs slow build #3455 #1438
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
Conversation
| S3Utils.getFile(s3, s3.getBucketName(), srcData.getPath(), destFile).waitForCompletion(); | ||
|
|
||
|
|
||
| if (destFile == null) { |
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.
I removed this because there is no way this "destFile" variable can be null
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.
Is this because new File(...) will always throw an exception if the value would be null?
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.
I removed that because the new File (...) will always create an object that is referenced by destFile; therefore, it will never be == null.
|
@swill @DaanHoogland tested this PR - LGTM.
Raja |
|
I don't think these tests failing are relevant to this change. We agree this is ready to be merged? Only against master? |
|
Code LGTM |
|
Tests were performed and reviews executed; so, should we merge this or the #1427? |
|
I think we should probably merge this one and not #1427. Do you agree? |
|
I agree, but I was the one that opened the PR, so in my opinion, my opinion should no count much here :) |
|
I opened #1427 and I agree, @rafaelweingartner and @swill so please go ahead. I will close #1427 |
Fix new error found in findbugs slow build #3455Fix new find bug error that was introduced in PR #1361 Report: http://jenkins.buildacloud.org/job/build-master-slowbuild/3455/findbugsResult/new/ It is the same fix as the one proposed in #1427; the difference is that this PR tried to change only the code that was strictly needed. However, I took the liberty to remove a dead code and few lines of code (annotations) that were not needed * pr/1438: Fix findbugs slow build 3455 Signed-off-by: Rafael Weingärtner <[email protected]>
Fix new find bug error that was introduced in PR #1361
Report: http://jenkins.buildacloud.org/job/build-master-slowbuild/3455/findbugsResult/new/
It is the same fix as the one proposed in #1427; the difference is that this PR tried to change only the code that was strictly needed. However, I took the liberty to remove a dead code and few lines of code (annotations) that were not needed