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

Split and Organize Checkstyle inputs by Test for checks in imports package #4437

Closed
subkrish opened this Issue Jun 8, 2017 · 12 comments

Comments

Projects
None yet
3 participants
@subkrish
Contributor

subkrish commented Jun 8, 2017

subtask of #3891

Move / Make a copy of all test input files to a separate folder for all checks in imports package.
Not any other Check should use new inputs.

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 8, 2017

Contributor

I'm on it. 1 test and its inputs per PR.

Contributor

subkrish commented Jun 8, 2017

I'm on it. 1 test and its inputs per PR.

@rnveach rnveach added the approved label Jun 8, 2017

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 8, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified AvoidStarImportCheckTest.java and moved its inp…
…ut files to the avoidstarimport subdirectory

rnveach added a commit that referenced this issue Jun 9, 2017

Issue #4437: Modified AvoidStarImportCheckTest.java and moved its inp…
…ut files to the avoidstarimport subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 9, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified AvoidStaticImportCheckTest.java and moved its i…
…nput files to the avoidstaticimport subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 9, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified CustomImportOrderCheckTest.java and moved its i…
…nput files to the customimportorder subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 9, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified CustomImportOrderCheckTest.java and moved its i…
…nput files to the customimportorder subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 9, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified IllegalImportCheckTest.java and moved its input…
… files to the illegalimport subdirectory

rnveach added a commit that referenced this issue Jun 9, 2017

Issue #4437: Modified AvoidStaticImportCheckTest.java and moved its i…
…nput files to the avoidstaticimport subdirectory

rnveach added a commit that referenced this issue Jun 9, 2017

Issue #4437: Modified IllegalImportCheckTest.java and moved its input…
… files to the illegalimport subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 9, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified CustomImportOrderCheckTest.java and moved its i…
…nput files to the customimportorder subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 12, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified CustomImportOrderCheckTest.java and moved its i…
…nput files to the customimportorder subdirectory

rnveach added a commit that referenced this issue Jun 12, 2017

Issue #4437: Modified CustomImportOrderCheckTest.java and moved its i…
…nput files to the customimportorder subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 15, 2017

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 16, 2017

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 19, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified UnusedImportsCheckTest.java and moved its input…
… files to the unusedimports subdirectory

rnveach added a commit that referenced this issue Jun 19, 2017

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 20, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified UnusedImportsCheckTest.java and moved its input…
… files to the unusedimports subdirectory

rnveach added a commit that referenced this issue Jun 20, 2017

Issue #4437: Modified UnusedImportsCheckTest.java and moved its input…
… files to the unusedimports subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 21, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 21, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 21, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 22, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 22, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory

romani added a commit that referenced this issue Jun 23, 2017

Issue #4437: Modified ImportControlCheckTest.java and moved its input…
… files to the importcontrol subdirectory
@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 23, 2017

Contributor

@romani @rnveach I was thinking about changing the input file for UnusedImportsCheckTest, so that once all the checks are arranged the extra files can be deleted.

I was planning to move InputImportBug to 'unusedimports' and change the test file a little accordingly.

Contributor

subkrish commented Jun 23, 2017

@romani @rnveach I was thinking about changing the input file for UnusedImportsCheckTest, so that once all the checks are arranged the extra files can be deleted.

I was planning to move InputImportBug to 'unusedimports' and change the test file a little accordingly.

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 23, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified RedundantImportCheckTest.java and moved its inp…
…ut files to the redundantimport subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 23, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified RedundantImportCheckTest.java and moved its inp…
…ut files to the redundantimport subdirectory
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 23, 2017

Member

I was planning to move InputImportBug to 'unusedimports' and change the test file a little accordingly.

lets not change content in scope of this issue. Lets finish just moving and then do some changes to content.

once all the checks are arranged the extra files can be deleted.

if we have extra input files that nobody use - please remove them in separate PR.

Member

romani commented Jun 23, 2017

I was planning to move InputImportBug to 'unusedimports' and change the test file a little accordingly.

lets not change content in scope of this issue. Lets finish just moving and then do some changes to content.

once all the checks are arranged the extra files can be deleted.

if we have extra input files that nobody use - please remove them in separate PR.

rnveach added a commit that referenced this issue Jun 24, 2017

Issue #4437: Modified RedundantImportCheckTest.java and moved its inp…
…ut files to the redundantimport subdirectory
@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 24, 2017

Contributor

lets not change content in scope of this issue. Lets finish just moving and then do some changes > to content.

Should I just make PR stating a minor change in input files after this issue is closed?

Contributor

subkrish commented Jun 24, 2017

lets not change content in scope of this issue. Lets finish just moving and then do some changes > to content.

Should I just make PR stating a minor change in input files after this issue is closed?

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 24, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlLoaderTest.java and moved its inpu…
…t files to the importcontrolloader subdirectory

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 24, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Modified ImportControlLoaderTest.java and moved its inpu…
…t files to the importcontrolloader subdirectory
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 24, 2017

Member

@subkrish , yes, separate PR.

Member

romani commented Jun 24, 2017

@subkrish , yes, separate PR.

romani added a commit that referenced this issue Jun 25, 2017

Issue #4437: Modified ImportControlLoaderTest.java and moved its inpu…
…t files to the importcontrolloader subdirectory
@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 25, 2017

Member

there 4 files left:

$ ls --group-directories-first | grep "\."
import-control_one-re.xml
import-control_one.xml
InputAvoidStaticImportNestedClass.java
InputImportBug.java
Member

romani commented Jun 25, 2017

there 4 files left:

$ ls --group-directories-first | grep "\."
import-control_one-re.xml
import-control_one.xml
InputAvoidStaticImportNestedClass.java
InputImportBug.java
@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 25, 2017

Contributor

@romani Yes there are 4 files left. I will check if any files are using them or delete them in the final PR for this issue. If any files are using some inputs because of which I would need to alter the input file I will handle them in another minor PR outside this issue.

Contributor

subkrish commented Jun 25, 2017

@romani Yes there are 4 files left. I will check if any files are using them or delete them in the final PR for this issue. If any files are using some inputs because of which I would need to alter the input file I will handle them in another minor PR outside this issue.

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 26, 2017

Contributor

@romani @rnveach The 4 files are used in different places.

import-control_one.xml and import-control_one-re.xml

Used by CheckerTest at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java#L683 and at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java#L711.

InputAvoidStaticImportNestedClass.java

Used by InputAvoidStaticImportDefault.java and AvoidStaticImportCheckTest.java.

InputImportBug.java

Used by InputImportOrder_WildcardUnspecified.java, InputUnusedImports.java, RedundantImportCheckTest.java and UnusedImportsCheckTest.java

They are accessed from that directory. Should I just create a 'minor' PR outside this issue and make the necessary changes in the test files or the input files or should it be done in this issue itself?

Contributor

subkrish commented Jun 26, 2017

@romani @rnveach The 4 files are used in different places.

import-control_one.xml and import-control_one-re.xml

Used by CheckerTest at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java#L683 and at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/CheckerTest.java#L711.

InputAvoidStaticImportNestedClass.java

Used by InputAvoidStaticImportDefault.java and AvoidStaticImportCheckTest.java.

InputImportBug.java

Used by InputImportOrder_WildcardUnspecified.java, InputUnusedImports.java, RedundantImportCheckTest.java and UnusedImportsCheckTest.java

They are accessed from that directory. Should I just create a 'minor' PR outside this issue and make the necessary changes in the test files or the input files or should it be done in this issue itself?

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 26, 2017

Member

In scope of this issue please

Member

romani commented Jun 26, 2017

In scope of this issue please

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 26, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Arranged the remaining files from 'imports' directory an…
…d modified the test and input files based on the rearrangement

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 26, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Arranged the remaining files from 'imports' directory an…
…d modified the test and input files based on the rearrangement

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 27, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Arranged the remaining files from 'imports' directory an…
…d modified the test and input files based on the rearrangement

subkrish pushed a commit to subkrish/checkstyle that referenced this issue Jun 27, 2017

Subbu Dantu Subbu Dantu
Issue #4437: Arranged the remaining files from 'imports' directory an…
…d modified the test and input files based on the rearrangement

rnveach added a commit that referenced this issue Jun 27, 2017

Issue #4437: Arranged the remaining files from 'imports' directory an…
…d modified the test and input files based on the rearrangement
@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jun 27, 2017

Member

@subkrish @romani Please confirm all work is done.

Member

rnveach commented Jun 27, 2017

@subkrish @romani Please confirm all work is done.

@subkrish

This comment has been minimized.

Show comment
Hide comment
@subkrish

subkrish Jun 27, 2017

Contributor

@rnveach @romani Looks like all the files have been arranged and no files are left outside the subdirectory folders. I think the work on this issue is done.

Contributor

subkrish commented Jun 27, 2017

@rnveach @romani Looks like all the files have been arranged and no files are left outside the subdirectory folders. I think the work on this issue is done.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 28, 2017

Member

issue is done.

$ ls 
avoidstarimport    customimportorder  importcontrol        importorder      unusedimports
avoidstaticimport  illegalimport      importcontrolloader  redundantimport

@subkrish , thanks a lot for your contribution.

Member

romani commented Jun 28, 2017

issue is done.

$ ls 
avoidstarimport    customimportorder  importcontrol        importorder      unusedimports
avoidstaticimport  illegalimport      importcontrolloader  redundantimport

@subkrish , thanks a lot for your contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment