Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Fix new error found in findbugs slow build #3455#1438

Merged
asfgit merged 1 commit intoapache:masterapache/cloudstack:masterfrom
rafaelweingartner:fixFindBugsBuild3455rafaelweingartner/cloudstack:fixFindBugsBuild3455Copy head branch name to clipboard
Mar 31, 2016
Merged

Fix new error found in findbugs slow build #3455#1438
asfgit merged 1 commit intoapache:masterapache/cloudstack:masterfrom
rafaelweingartner:fixFindBugsBuild3455rafaelweingartner/cloudstack:fixFindBugsBuild3455Copy head branch name to clipboard

Conversation

@rafaelweingartner
Copy link
Member

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

S3Utils.getFile(s3, s3.getBucketName(), srcData.getPath(), destFile).waitForCompletion();


if (destFile == null) {
Copy link
Member Author

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

Copy link
Contributor

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?

Copy link
Member Author

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.

@rajap9711
Copy link

@swill @DaanHoogland tested this PR - LGTM.
All BVTs have passed with the exception of the following tests

  • test_07_list_default_iso - iso didn't exist.
  • test_01_test_vm_volume_snapshot - test case issue - operation is not allowed?

Raja

@swill
Copy link
Contributor

swill commented Mar 25, 2016

I don't think these tests failing are relevant to this change. We agree this is ready to be merged? Only against master?

@nvazquez
Copy link
Contributor

Code LGTM

@rafaelweingartner
Copy link
Member Author

Tests were performed and reviews executed; so, should we merge this or the #1427?

@swill
Copy link
Contributor

swill commented Mar 31, 2016

I think we should probably merge this one and not #1427. Do you agree?

@rafaelweingartner
Copy link
Member Author

I agree, but I was the one that opened the PR, so in my opinion, my opinion should no count much here :)

@DaanHoogland
Copy link
Contributor

I opened #1427 and I agree, @rafaelweingartner and @swill so please go ahead. I will close #1427

@asfgit asfgit merged commit b3de01a into apache:master Mar 31, 2016
asfgit pushed a commit that referenced this pull request Mar 31, 2016
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 <rafael@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.