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

Move linter to Jenkins. #4032

Closed
trivialfis opened this issue Dec 29, 2018 · 4 comments
Closed

Move linter to Jenkins. #4032

trivialfis opened this issue Dec 29, 2018 · 4 comments

Comments

@trivialfis
Copy link
Member

trivialfis commented Dec 29, 2018

Currently the linter is a good hack, but not reliable due to the fact that clang-tidy doesn't deal well with CUDA source code. On Travis our linter script renames all cu files into cc files and perform clang-tidy checks, which leads to incorrect results, see the end of:

https://travis-ci.org/dmlc/xgboost/jobs/473251478

Now we have CUDA runtime on Jenkins thanks to @hcho3 , where we can use clang-tidy without such hack. The correct clang-tidy results I generated is attached.

log-tidy.txt

@trivialfis
Copy link
Member Author

Regenerated with both host and device checking:
log-tidy.txt

@trivialfis
Copy link
Member Author

trivialfis commented Dec 29, 2018

Requirements:

  • clang-tidy >= 7
  • A huge refactor of current code base.
  • CMake >= 3.8
  • Extra script on CMake to obtain the compilation database.

@RAMitchell @hcho3 Do you think it's worthy?

@RAMitchell
Copy link
Member

You dont have to refactor the lint errors all in one go. What if you set up the system on jenkins but allow it to pass even with errors, spend a couple of weeks fixing lint issues, then make it mandatory.

I think this is very much worth doing.

@trivialfis
Copy link
Member Author

Closing. PR is here #4034 .

@lock lock bot locked as resolved and limited conversation to collaborators Mar 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants