mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Vinod Kone" <vinodk...@gmail.com>
Subject Re: Review Request 40445: Added linter for license headers in some file types.
Date Tue, 24 Nov 2015 19:08:48 GMT


> On Nov. 18, 2015, 6:21 p.m., Vinod Kone wrote:
> > support/hooks/post-rewrite, line 33
> > <https://reviews.apache.org/r/40445/diff/1/?file=1130933#file1130933line33>
> >
> >     instead of calling this script directly, i would recommend calling it from mesos-style.py.
this way users and tools (review bot) can run just one script to test compliance.
> 
> Benjamin Bannier wrote:
>     We do already call two scripts from the hooks ATM (one to wrap `cpplint.py`, and
one to check for properly split commits across libraries). Looking at `mesos-style.py`, its
logic is pretty much tied to just running `cpplint` and parsing its output -- do you really
mean we should rewrite all of that?
>     
>     Note that users can always manually run the hooks.

I just meant to call mesos-license.py from mesos-style.py because it is essentially a style
checker. This just adds ~1 line to mesos-style.py. The reason I suggested this because if
there are multiple scripts to run to check conformance, it will be hard to track. For example,
review bot manually runs `./support/mesos-style.py` before doing `make check`. I would like
to avoid updating review bot, everytime someone adds a new script to improve style checking.
It would be also easy for  us to tell users to run this single script to check sytle (they
might want to check before doing commit). 

The reason split commits checker has its own line in the commit hook is because it only comes
into play when committing. For example, you wouldn't (couldn't) run that script on a freshly
cloned repo. This is the same reason review bot doesn't run that checker.

Does that make sense?


- Vinod


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40445/#review107061
-----------------------------------------------------------


On Nov. 18, 2015, 4:05 p.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40445/
> -----------------------------------------------------------
> 
> (Updated Nov. 18, 2015, 4:05 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman and Marco Massenzio.
> 
> 
> Bugs: MESOS-3581
>     https://issues.apache.org/jira/browse/MESOS-3581
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added linter for license headers in some file types.
> 
> 
> Diffs
> -----
> 
>   support/hooks/post-rewrite 7df1e0f29c6ce940a364c0b1d312251c6160e5e3 
>   support/hooks/pre-commit ca9e9810aca921734be5224e3ef71fe7ff4aa03d 
>   support/mesos-license.py PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/40445/diff/
> 
> 
> Testing
> -------
> 
> Ran the a whole clean checkout through the linter with only one expected failure (`3rdparty/libprocess/stout/tests/protobuf_tests.proto`
which lacks a license).
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message