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 #4365: Split and Organize Checkstyle inputs by Test for checks … #4424

Closed
wants to merge 24 commits into from
Closed

Conversation

sharang108
Copy link
Contributor

@sharang108 sharang108 commented Jun 6, 2017

Solution for Issue #4365

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.

@rnveach
Copy link
Member

rnveach commented Jun 6, 2017

@sharang108
CI will need to pass before we start reviewing. Right now your tests are failing.

@sharang108
Copy link
Contributor Author

sharang108 commented Jun 6, 2017

@rnveach The local test mvn clean verify is showing build successful, Log

@rnveach
Copy link
Member

rnveach commented Jun 6, 2017

@sharang108 We can't go by your local as we can't see it. We can only go by what our CIs say.
Start by confirming that you have committed and pushed everything from your repository.

@sharang108
Copy link
Contributor Author

sharang108 commented Jun 6, 2017

@rnveach Well you've said that if the changes pass the local test(s), then I can commit them and create pull request for the related issue. Anyway what should I do next? I don't understand the error Logs on the CIs.

@rnveach
Copy link
Member

rnveach commented Jun 6, 2017

Anyway what should I do next?

Did you confirm that you have committed and pushed everything from your repository?

I don't understand the error Logs on the CIs.

https://travis-ci.org/checkstyle/checkstyle/jobs/239900704#L647

Caused by: com.puppycrawl.tools.checkstyle.api.CheckstyleException: Unable to find: /home/travis/build/checkstyle/checkstyle/src/test/resources/com/puppycrawl/tools/checkstyle/checks/header/regexpheader/regexp.header5

After some looking, problem is because your branch is out of date. We added new tests and inputs, and your PR doesn't move those files so it can't find them.
Please rebase on the latest master.

voidfist and others added 23 commits June 7, 2017 00:16
… this commit will be revered after fix for #4387"

This reverts commit e88cdd4.
…pends on old version and could cause problems
@codecov-io
Copy link

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #4424   +/-   ##
=======================================
  Coverage   99.99%   99.99%           
=======================================
  Files         285      285           
  Lines       15293    15293           
  Branches     3478     3478           
=======================================
  Hits        15292    15292           
  Partials        1        1

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 6e84fdf...21706f7. Read the comment docs.

@sharang108 sharang108 closed this Jun 6, 2017
sergileon added a commit to sergileon/checkstyle that referenced this pull request Sep 5, 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

6 participants