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

Issue #3959: Split and Organize Checkstyle inputs by Test for FileTabCharacter #4182

Merged
merged 1 commit into from Apr 12, 2017

Conversation

Kietzmann
Copy link
Contributor

This pull request covers to issue #3959
What was modified:

Modified paths to test classes
Moved all necessary inputs into /whitespace/filltabcharacter package
Results:

All unit tests passed.
the result of mvn clean verify also OK

@codecov-io
Copy link

codecov-io commented Apr 10, 2017

Codecov Report

Merging #4182 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #4182   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         283     283           
  Lines       14880   14880           
  Branches     3401    3401           
======================================
  Hits        14880   14880

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4cef3f9...8e6da6e. Read the comment docs.

@@ -56,9 +58,9 @@ public void testDefault() throws Exception {
"19:25: " + getCheckMessage(MSG_FILE_CONTAINS_TAB),
};
final File[] files = {
new File(getPath("InputSimple.java")),
new File(getPath("SimpleFileTabCharacterInput.java")),
Copy link
Member

Choose a reason for hiding this comment

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

Input files should be named after their tests.
FileTabCharacterCheckTest's inputs should be named InputFileTabCharacter....

* - Order of modifiers
* @author Oliver Burn
**/
final class SimpleFileTabCharacterInput
Copy link
Member

Choose a reason for hiding this comment

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

input classes must match file name.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Recently I made a commit with appropriate changes

Copy link
Member

Choose a reason for hiding this comment

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

@Kietzmann This is still not done.

@romani
Copy link
Member

romani commented Apr 11, 2017

@Kietzmann , please squash all commits in one for final review.

@Kietzmann
Copy link
Contributor Author

@rnveach
Please make code review.
Thanks.

@Vladlis
Copy link
Contributor

Vladlis commented Apr 11, 2017

@Kietzmann
Please address @rnveach's point and rename class SimpleFileTabCharacterInput to InputFileTabCharacterSimple.

Please, write a proper commit message,

@Kietzmann
Copy link
Contributor Author

@Vladlis
Actually I've hastened with previous commit message. But I've already renamed SimpleFileTabCharacterInput to InputFileTabCharacterSimple.

@Vladlis
Copy link
Contributor

Vladlis commented Apr 11, 2017

@Kietzmann
You have only renamed the file, but class name is still SimpleFileTabCharacterInput

@Kietzmann
Copy link
Contributor Author

Ups...
Now it seems that everything correct

@rnveach
Copy link
Member

rnveach commented Apr 11, 2017

@Kietzmann Now you need to rebase on the latest master. When you squashed the commits, somehow you added commits that were not your own so now you have changes outside of FileTabCharacter.
We only expect changes to FileTabCharacterTest and it's inputs.
Please review your commit before pushing again.

@Kietzmann
Copy link
Contributor Author

@rnveach Seems to be fixed

@rnveach rnveach requested a review from romani April 11, 2017 11:51
@romani romani merged commit cc89339 into checkstyle:master Apr 12, 2017
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.

None yet

5 participants