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 header package #4365

Closed
romani opened this Issue May 20, 2017 · 10 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented May 20, 2017

subtask of #3891

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

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani May 20, 2017

Member

@sharang108, please help us with this issue.
You can find bunch of example on how to do this (in parent issue there are refercences to commits from others for similar task)

Member

romani commented May 20, 2017

@sharang108, please help us with this issue.
You can find bunch of example on how to do this (in parent issue there are refercences to commits from others for similar task)

@sharang108

This comment has been minimized.

Show comment
Hide comment
@sharang108

sharang108 May 20, 2017

Contributor

@romani I am on it!

Contributor

sharang108 commented May 20, 2017

@romani I am on it!

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 6, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 7, 2017

Issue #4365: Split and Organize Checkstyle inputs by Test for checks …
…in header package

Issue #4365: Split and Organize Checkstyle inputs by Test for checks in header package

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 7, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 9, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 12, 2017

y# This is a combination of 2 commits.
Issue #4365: Split and Organize Checkstyle inputs by Test for checks in header package

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 12, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 16, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 16, 2017

sharang108 added a commit to sharang108/checkstyle that referenced this issue Jun 16, 2017

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

@rnveach

This comment has been minimized.

Show comment
Hide comment
Member

rnveach commented Jun 20, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jun 30, 2017

Member

@sharang108 , can you help us to finish this issue ?
two input files left.

Member

romani commented Jun 30, 2017

@sharang108 , can you help us to finish this issue ?
two input files left.

@sharang108

This comment has been minimized.

Show comment
Hide comment
@sharang108

sharang108 Jul 3, 2017

Contributor

@romani which files?

Contributor

sharang108 commented Jul 3, 2017

@romani which files?

@romani

This comment has been minimized.

Show comment
Hide comment

Kietzmann added a commit to Kietzmann/checkstyle that referenced this issue Jul 9, 2017

Kietzmann pushed a commit to Kietzmann/checkstyle that referenced this issue Jul 10, 2017

romani added a commit that referenced this issue Jul 10, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
Member

romani commented Jul 10, 2017

@romani romani closed this Jul 10, 2017

@romani romani added this to the 8.1 milestone Jul 10, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Jul 10, 2017

Member

@romani I am re-opening this issue.
One part was done, moving files. What wasn't done was renaming files to be named after their tests.

https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/header/header
InputRegexpHeader.java should be named InputHeaderRegexp.java or any other variant.
@Kietzmann Please provide us with a fix.

I also question the usage of getConfigPath at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java#L64
We are storing extra inputs at https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/configs which is outside the directory of the other inputs.
Shouldn't these files be moved with the other inputs and named accordingly?
@romani Please confirm this point if we should do this or not.

Member

rnveach commented Jul 10, 2017

@romani I am re-opening this issue.
One part was done, moving files. What wasn't done was renaming files to be named after their tests.

https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/checks/header/header
InputRegexpHeader.java should be named InputHeaderRegexp.java or any other variant.
@Kietzmann Please provide us with a fix.

I also question the usage of getConfigPath at https://github.com/checkstyle/checkstyle/blob/master/src/test/java/com/puppycrawl/tools/checkstyle/checks/header/HeaderCheckTest.java#L64
We are storing extra inputs at https://github.com/checkstyle/checkstyle/tree/master/src/test/resources/com/puppycrawl/tools/checkstyle/configs which is outside the directory of the other inputs.
Shouldn't these files be moved with the other inputs and named accordingly?
@romani Please confirm this point if we should do this or not.

@rnveach rnveach reopened this Jul 10, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 10, 2017

Member

ok, lets finish this
getConfigPath should be removed to force Test to use only one folder for all files.

Member

romani commented Jul 10, 2017

ok, lets finish this
getConfigPath should be removed to force Test to use only one folder for all files.

djydewang added a commit to djydewang/checkstyle that referenced this issue Jul 11, 2017

djydewang added a commit to djydewang/checkstyle that referenced this issue Jul 11, 2017

romani added a commit that referenced this issue Jul 11, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Jul 11, 2017

Member

final fix is provided

Member

romani commented Jul 11, 2017

final fix is provided

@romani romani closed this Jul 11, 2017

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