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.

Sign up for GitHub

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

Jump to bottom

MapIterationInForEachLoopCheck + CustomDeclarationOrderCheck #294

Closed
wants to merge 5 commits into from
Closed

MapIterationInForEachLoopCheck + CustomDeclarationOrderCheck #294

wants to merge 5 commits into from

Conversation

damianszczepanik
Copy link
Member

MapIterationInForEachLoopCheck

  • Converted strings to .class.getName()
  • moved initialization to beginTree method

CustomDeclarationOrderCheck

  • Added tests to increase coverage
  • Fixed rule in test method so more violations are raised
  • Fixed formatting (indents)

@damianszczepanik
Verifies that AutoCloseable resource is declared with try-with-resour…
a4f0148 
…ces (JDK 1.7+) statement to make sure that is closed correctly.

- Added Check class for validation
- Prepared Input classes including those that work from JDK7
- Added violation to messages.properties
- added resources-noncompilance to eclipse project to simplify development
@damianszczepanik
- Added tests to improve coverage
047d5c3 
- Improved javadoc
- Updated configuration files - created check/rule is added to xml files
@damianszczepanik
- Added tests with diamond operator
761d800
@coveralls
Copy link

Coverage Status

Coverage increased (+1.19%) when pulling 33a9ac0 on damianszczepanik:clearing-imports2 into d2c4c45 on sevntu-checkstyle:master.

@damianszczepanik
Copy link
Member Author

This PR does not refer to issue or request so what tag shall I add to message? I see that some commits has simple information bout change but without id.

@romani
Copy link
Member

romani commented Jan 5, 2015

please put "#294" some where in message, to let easily find a reason on that update in future.

@romani
Copy link
Member

romani commented Jan 5, 2015

please split changes to each Check in separate PR, changes are not related to each other so should be separate.
Please also explain a reason of changes in InputOrder.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0%) when pulling 7ae9139 on damianszczepanik:clearing-imports2 into d2c4c45 on sevntu-checkstyle:master.

@romani
Copy link
Member

romani commented Feb 2, 2015

@damianszczepanik , please redo that PR.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 96.53% when pulling 68f5c06 on damianszczepanik:clearing-imports2 into 16e829e on sevntu-checkstyle:master.

@damianszczepanik
MapIterationInForEachLoopCheck
e19ea3c 
- Converted strings to .class.getName()
- moved initialization to beginTree method

Pull #294
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 96.53% when pulling e19ea3c on damianszczepanik:clearing-imports2 into 16e829e on sevntu-checkstyle:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.09%) to 96.53% when pulling e19ea3c on damianszczepanik:clearing-imports2 into 16e829e on sevntu-checkstyle:master.

@damianszczepanik
Copy link
Member Author

Sorry for delay. I thought it was already merged.
Coverage decreases because I've added one method but I'm not sure if I rebased correctly...

@romani
Copy link
Member

romani commented Feb 4, 2015

almost all outside PR are never appied without additonal changes :).

@damianszczepanik , your branch (https://github.com/damianszczepanik/sevntu.checkstyle/commits/clearing-imports2) still consist changes that are not related to your issue, please fix.

Please create new branch and possibly new PR that will contain only meaningful changes that you are going to introduce. I recoomend you to split changes to each Check to separate PR to simplify acceptance process.

@damianszczepanik
Copy link
Member Author

Sorry, for the problem. New PR is waiting for approval.

@damianszczepanik damianszczepanik deleted the clearing-imports2 branch February 4, 2015 19:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
incomplete
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@damianszczepanik @coveralls @romani