Skip to content

Extend tidy license check to remaining source code #4534

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

Closed
brson opened this issue Jan 18, 2013 · 8 comments · Fixed by #12243
Closed

Extend tidy license check to remaining source code #4534

brson opened this issue Jan 18, 2013 · 8 comments · Fixed by #12243
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority

Comments

@brson
Copy link
Contributor

brson commented Jan 18, 2013

make tidy checks the license of files but tidy is only run on a subset of the repository since some third party, tests and overlooked code isn't checked for formatting. We need to extend the tidy check to all source code, even tests (I think).

@catamorphism
Copy link
Contributor

Another thing to add to the tidy license checking: checking the copyright year that appears at the top of the file. Graydon said this should be a list or range of years in which significant changes were made. So many files will have to change from 2012 (as they are now) to 2011-2013 or some other such thing.

@graydon
Copy link
Contributor

graydon commented May 1, 2013

I think this is done now, no?

@catamorphism
Copy link
Contributor

@graydon It seems like it's not -- tidy isn't being run on tests (so that tests aren't required to have the 100-column limit), but tidy calls the license script. So the tests aren't being checked for licenses at all. Unless I'm missing something.

@catamorphism
Copy link
Contributor

Nominating for milestone 5, production-ready -- we probably have to bite the bullet and do this sooner than later.

@catamorphism
Copy link
Contributor

Accepted for milestone 5

@dguenther
Copy link
Contributor

Once #12055 lands, are there any other areas that need checking? Right now it appears we run tidy on:

  • *.r[sc] except in libuv, llvm, and gyp
  • *.cpp in rt, rt/*, rt/*/*, and rustllvm except miniz.cpp and in rt/sundown
  • *.h in rt, rt/*, rt/*/*, and rustllvm except rt/vg/valgrind.h, rt/vg/memcheck.h, rt/msvc/typeof.h, rt/msvc/stdint.h, rt/msvc/inttypes.h, and in rt/sundown
  • *.py in etc

@pnkfelix
Copy link
Member

pnkfelix commented Feb 6, 2014

P-low. This issue is almost done, according to @brson.

@brson
Copy link
Contributor Author

brson commented Feb 6, 2014

@dguenther The only remaining missing licenses I'm aware of are in src/doc. There are some scripts there that should have the license block, but more importantly the docs themselves do not have a license declared but should (#12069). I'd consider the doc licenses to be a separate issue, so once the scripts are all covered this issue is done.

bors added a commit that referenced this issue Feb 7, 2014
This PR extends the tidy formatting check to rust files in the test folder. To facilitate this, a few flags were added to tidy:

* `xfail-tidy-cr` - Disables the check for CR characters for all following lines in the file
* `xfail-tidy-tab` - Disables the check for tab characters for all following lines in the file
* `xfail-tidy-linelength` - Disables the line length check for all following lines in the file

Checks should not have to be disabled often. I disabled line length checks in `debug-info` tests that use `debugger:` checks, but aside from that, there were relatively few exclusions. Running tidy on all the tests does slow down the formatting check, so it may be worth investigating further optimization.

cc #4534
bors added a commit that referenced this issue Feb 17, 2014
Extends the license and formatting check to `*.js` files in `src/doc` and `*.sh`, `*.pl`, `*.c`, and `*.h` files in `src/etc`. As best as I could tell, these files should be covered under the Rust project license.

cc @brson: Do any other scripts need a license? I'd like to double-check that this PR closes #4534.
@bors bors closed this as completed in 03c5342 Feb 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-testsuite Area: The testsuite used to check the correctness of rustc E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. P-low Low priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants