Skip to content
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

#624 - Removing 'excludes' of FindBuging #647

Merged

Conversation

dalifreire
Copy link
Contributor

Issue #624 - Solving puzzle 574-2964594e in pom.xml.

Fixing and removing the following 'excludes' of FindBugs in the pom.xml file:

  • findbugs:org.takes.facets.fork.FkRegex$2 SIC_INNER_SHOULD_BE_STATIC_ANON
  • findbugs:org.takes.http.FtCLI$3 SIC_INNER_SHOULD_BE_STATIC_ANON
  • findbugs:org.takes.http.MainRemote$1 SIC_INNER_SHOULD_BE_STATIC_ANON
  • findbugs:org.takes.rq.RqMultipart$Base VA_FORMAT_STRING_USES_NEWLINE

30 minutes to implement this task. It is not enough because the problem is much bigger and requires more work than the slotted time allows. A new puzzle was created so that can be resolved by other people.

@dalifreire dalifreire closed this Mar 9, 2016
@dalifreire dalifreire reopened this Mar 11, 2016
@dalifreire
Copy link
Contributor Author

@davvd Could you find someone to review it?

@dalifreire dalifreire changed the title #624 - Removes all excludes of FindBugs. #624 - Removes 'excludes' of FindBugs. Mar 11, 2016
@dalifreire dalifreire changed the title #624 - Removes 'excludes' of FindBugs. #624 - Removing 'excludes' of FindBuging Mar 11, 2016
@davvd
Copy link

davvd commented Mar 11, 2016

@dalifreire Many thanks for the PR, let me find a reviewer for it

@davvd
Copy link

davvd commented Mar 11, 2016

@carlosmiranda review this one, please

@carlosmiranda
Copy link

@dalifreire looks quite good, thanks!

@carlosmiranda
Copy link

@rultor merge please

@rultor
Copy link
Collaborator

rultor commented Mar 14, 2016

@rultor merge please

@carlosmiranda Thanks for your request. @yegor256 Please confirm this.

} catch (final InvocationTargetException ex) {
throw new IllegalStateException(ex);
} catch (final IllegalAccessException ex) {
throw new IllegalStateException(ex);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 Done!

@yegor256
Copy link
Owner

@dalifreire one comment above

Throwing an Exception Without Proper Context Is a Bad Habit
@dalifreire
Copy link
Contributor Author

@carlosmiranda @yegor256 Thanks for review. Could you take a look if everything is right now?

this.method.getName()
),
ex
);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dalifreire this indentation looks strange to me :) and a few lines above too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yegor256 Fixed, thanks! :)

@yegor256
Copy link
Owner

@dalifreire looks good to me, but there is something wrong with indentation, see above

@dalifreire
Copy link
Contributor Author

@yegor256 Indentation fixed! Thanks! Please, check and merge...

@yegor256
Copy link
Owner

@rultor try to merge

@rultor
Copy link
Collaborator

rultor commented Mar 17, 2016

@rultor try to merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor
Copy link
Collaborator

rultor commented Mar 17, 2016

@rultor try to merge

@dalifreire @yegor256 Oops, I failed. You can see the full log here (spent 50min)

+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2466: running'
waiting for AppVeyor build 2466: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2466
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2466: running'
waiting for AppVeyor build 2466: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2466
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2466: running'
waiting for AppVeyor build 2466: running
+ sleep 5s
+ true
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2466
++ jq -r .build.status
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2466: running'
waiting for AppVeyor build 2466: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2466
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2466: running'
waiting for AppVeyor build 2466: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2466
+ status=failed
+ '[' failed == null ']'
+ '[' failed == success ']'
+ '[' failed == failed ']'
+ echo 'see https://ci.appveyor.com/project/yegor256/takes/build/2466'
see https://ci.appveyor.com/project/yegor256/takes/build/2466
+ exit 1
container 6da7106ef503e04ebe4711b307751144bb41a18cea14130d20fa18f47e0c9348 is dead
Thu Mar 17 05:21:11 UTC 2016

@yegor256
Copy link
Owner

@rultor try to merge again

@rultor
Copy link
Collaborator

rultor commented Mar 17, 2016

@rultor try to merge again

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@yegor256
Copy link
Owner

@dalifreire I can't "check", you have to create a ticket if you think that the project is broken, see http://www.yegor256.com/2015/01/15/how-to-cut-corners.html

@rultor
Copy link
Collaborator

rultor commented Mar 17, 2016

@rultor try to merge again

@dalifreire @yegor256 Oops, I failed. You can see the full log here (spent 1hr)

+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2473: running'
waiting for AppVeyor build 2473: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2473
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2473: running'
waiting for AppVeyor build 2473: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2473
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2473: running'
waiting for AppVeyor build 2473: running
+ sleep 5s
+ true
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2473
++ jq -r .build.status
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2473: running'
waiting for AppVeyor build 2473: running
+ sleep 5s
+ true
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2473
++ jq -r .build.status
+ status=running
+ '[' running == null ']'
+ '[' running == success ']'
+ '[' running == failed ']'
+ echo 'waiting for AppVeyor build 2473: running'
waiting for AppVeyor build 2473: running
+ sleep 5s
+ true
++ jq -r .build.status
++ curl -K ../curl-appveyor.cfg https://ci.appveyor.com/api/projects/yegor256/takes/build/2473
+ status=failed
+ '[' failed == null ']'
+ '[' failed == success ']'
+ '[' failed == failed ']'
+ echo 'see https://ci.appveyor.com/project/yegor256/takes/build/2473'
see https://ci.appveyor.com/project/yegor256/takes/build/2473
+ exit 1
container 50568a4512323429c1cbae95c6191b37ddbb3d4c00ac423519e800b57ebb9e04 is dead
Thu Mar 17 18:45:38 UTC 2016

@dalifreire dalifreire mentioned this pull request Mar 18, 2016
@carlosmiranda
Copy link

@dalifreire any updates here?

@dalifreire
Copy link
Contributor Author

@carlosmiranda I'm waiting for the merge... take a look here: #654

@carlosmiranda
Copy link

@rultor merge

@rultor
Copy link
Collaborator

rultor commented Mar 30, 2016

@rultor merge

@carlosmiranda Thanks for your request. @yegor256 Please confirm this.

@dalifreire
Copy link
Contributor Author

@yegor256 ping... @davvd is punishing me for delay.... =)

@dalifreire
Copy link
Contributor Author

@yegor256 Please, could you confirm the merge?

@yegor256
Copy link
Owner

@rultor try to merge

@rultor
Copy link
Collaborator

rultor commented Apr 11, 2016

@rultor try to merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

@rultor rultor merged commit 57752b4 into yegor256:master Apr 11, 2016
@rultor
Copy link
Collaborator

rultor commented Apr 11, 2016

@rultor try to merge

@yegor256 Done! FYI, the full log is here (took me 12min)

@davvd
Copy link

davvd commented Apr 12, 2016

@ypshenychka please, let us know what do you think about this ticket, according to our QA rules

@davvd
Copy link

davvd commented Apr 12, 2016

@rultor deploy now pls

@rultor
Copy link
Collaborator

rultor commented Apr 12, 2016

@rultor deploy now pls

@davvd OK, I'll try to deploy now. You can check the progress here

@ypshenychka
Copy link

@carlosmiranda
According to our QA Rules:

The code reviewer found at least three problems in the code.

No issues were found during code review.
Please confirm that you'll try to find at least three problems while future reviews.

@rultor
Copy link
Collaborator

rultor commented Apr 12, 2016

@rultor deploy now pls

@davvd Done! FYI, the full log is here (took me 17min)

@carlosmiranda
Copy link

@ypshenychka confirmed, thanks

@ypshenychka
Copy link

@carlosmiranda Thanks.

@ypshenychka
Copy link

@davvd Quality is acceptable.

@davvd
Copy link

davvd commented Apr 13, 2016

@davvd Quality is acceptable.

@ypshenychka thanks for the QA review, we'll work better next time

@davvd
Copy link

davvd commented Apr 13, 2016

@carlosmiranda 10 mins was added to the account of @ypshenychka (for QA review), in transaction 83433697. Many thanks! 20 mins were added to your account in Transaction ID 83433716 (task took 745 hours and 25 mins). review comments (c=5) added as a bonus. +20 to your rating, your total score is +2316

@dalifreire dalifreire deleted the #624_remove-excludes-findbugs-violations branch April 13, 2016 22:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants