You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Cody Maloney <co...@mesosphere.io> on 2014/10/16 23:04:14 UTC
Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/
-----------------------------------------------------------
Review request for mesos, Adam B and Vinod Kone.
Bugs: MESOS-1712 and MESOS-1932
https://issues.apache.org/jira/browse/MESOS-1712
https://issues.apache.org/jira/browse/MESOS-1932
Repository: mesos-git
Description
-------
Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
Sample output:
```
ERROR: Commit spanning multiple projects
Mesos rules state that a commit should only touch one mesos project (stout,
libprocess, mesos).
Paths grouped by project:
mesos:
baz
libprocess:
3rdparty/libprocess/asdf
stout:
3rdparty/libprocess/3rdparty/stout/foobar
```
Diffs
-----
support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
support/mesos_split.py PRE-CREATION
Diff: https://reviews.apache.org/r/26825/diff/
Testing
-------
Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
Thanks,
Cody Maloney
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57056
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26825]
All tests passed.
- Mesos ReviewBot
On Oct. 16, 2014, 9:04 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:04 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57687
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26825]
All tests passed.
- Mesos ReviewBot
On Oct. 21, 2014, 9:20 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 9:20 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Cody Maloney <co...@mesosphere.io>.
> On Oct. 22, 2014, 8:44 a.m., Adam B wrote:
> > support/mesos_split.py, line 63
> > <https://reviews.apache.org/r/26825/diff/2/?file=728014#file728014line63>
> >
> > Did you intend to have two spaces before the %s? I would think a single space would be enough.
The goal is that they are visibly indented. One would probably work, I went with two since that is our standard indent.
> On Oct. 22, 2014, 8:44 a.m., Adam B wrote:
> > support/mesos_split.py, line 27
> > <https://reviews.apache.org/r/26825/diff/2/?file=728014#file728014line27>
> >
> > Don't you need to have at least 2 filenames for this script to make any sense?
A commit can contain just a single file, and the script needs to function correctly in that case.
> On Oct. 22, 2014, 8:44 a.m., Adam B wrote:
> > support/mesos_split.py, line 28
> > <https://reviews.apache.org/r/26825/diff/2/?file=728014#file728014line28>
> >
> > Maybe make it more explicit how the filenames should be delimited (space) by suggesting "Usage: ./mesos-split.py <filename1> <filename2>..."
The angle bracket notation is fairly standard.
git add --help:
git add [-n] [-v] [--force | -f] [--interactive | -i] [--patch | -p]
[--edit | -e] [--[no-]all | --[no-]ignore-removal | [--update | -u]]
[--intent-to-add | -N] [--refresh] [--ignore-errors] [--ignore-missing]
[--] [<pathspec>...]
man ls:
ls [-ABCFGHLOPRSTUW@abcdefghiklmnopqrstuwx1] [file ...]
g++ --help:
Usage: g++ [options] file...
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57768
-----------------------------------------------------------
On Oct. 23, 2014, 5:17 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 5:17 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Adam B <ad...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57768
-----------------------------------------------------------
Looks great. A few minor suggestions/questions, and then I'd call it commit-worthy.
Will this also get run by reviewbot somehow?
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment98648>
Don't you need to have at least 2 filenames for this script to make any sense?
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment98647>
Maybe make it more explicit how the filenames should be delimited (space) by suggesting "Usage: ./mesos-split.py <filename1> <filename2>..."
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment98650>
Any reason why "3rdparty/libprocess/" ends in a '/' but stout doesn't? Consistency, please.
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment98649>
Did you intend to have two spaces before the %s? I would think a single space would be enough.
- Adam B
On Oct. 21, 2014, 2:20 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 2:20 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review58057
-----------------------------------------------------------
Patch looks great!
Reviews applied: [26825]
All tests passed.
- Mesos ReviewBot
On Oct. 23, 2014, 5:17 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 5:17 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review58305
-----------------------------------------------------------
Ship it!
Ship It!
- Vinod Kone
On Oct. 23, 2014, 5:17 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 23, 2014, 5:17 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/
-----------------------------------------------------------
(Updated Oct. 23, 2014, 5:17 p.m.)
Review request for mesos, Adam B and Vinod Kone.
Changes
-------
Update to address reviewer comments.
Bugs: MESOS-1712 and MESOS-1932
https://issues.apache.org/jira/browse/MESOS-1712
https://issues.apache.org/jira/browse/MESOS-1932
Repository: mesos-git
Description
-------
Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
Sample output:
```
ERROR: Commit spanning multiple projects
Mesos rules state that a commit should only touch one mesos project (stout,
libprocess, mesos).
Paths grouped by project:
mesos:
baz
libprocess:
3rdparty/libprocess/asdf
stout:
3rdparty/libprocess/3rdparty/stout/foobar
```
Diffs (updated)
-----
support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
support/mesos_split.py PRE-CREATION
Diff: https://reviews.apache.org/r/26825/diff/
Testing
-------
Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
Thanks,
Cody Maloney
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Cody Maloney <co...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/
-----------------------------------------------------------
(Updated Oct. 21, 2014, 9:20 p.m.)
Review request for mesos, Adam B and Vinod Kone.
Changes
-------
Address review comments
Bugs: MESOS-1712 and MESOS-1932
https://issues.apache.org/jira/browse/MESOS-1712
https://issues.apache.org/jira/browse/MESOS-1932
Repository: mesos-git
Description
-------
Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
Sample output:
```
ERROR: Commit spanning multiple projects
Mesos rules state that a commit should only touch one mesos project (stout,
libprocess, mesos).
Paths grouped by project:
mesos:
baz
libprocess:
3rdparty/libprocess/asdf
stout:
3rdparty/libprocess/3rdparty/stout/foobar
```
Diffs (updated)
-----
support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
support/mesos_split.py PRE-CREATION
Diff: https://reviews.apache.org/r/26825/diff/
Testing
-------
Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
Thanks,
Cody Maloney
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Cody Maloney <co...@mesosphere.io>.
> On Oct. 17, 2014, 12:54 a.m., Vinod Kone wrote:
> > support/mesos_split.py, line 27
> > <https://reviews.apache.org/r/26825/diff/1/?file=723339#file723339line27>
> >
> > It's weird that this script talks about commits but takes a list of files. Why not have it take it a commit sha and let it convert to a list of files?
>
> Cody Maloney wrote:
> The code is a lot simpler.
>
> To go from sha-1 back I would need to import subprocess, have it call git then parse the output. Having bash + xargs just take care of that is a lot simpler (And also matches the pre-commit style hook).
>
> Vinod Kone wrote:
> I agree about the simplicity. But the weird part for me is that it is violating abstractions. Somehow, the script knows that a list of files given to it belongs to a commit and thats what its error message says. Maybe the error meessage need not mention commit? But we still need to say it is the commit that is in violation so that we give a better error message to ppl using it. Hmm.
>
> Alternatively, do you think this script would be used for any reason other than to examine a commit? If not, I think it should take a commit.
Making it take a commit is hard, esp. in the pre-commit hook. At that point, there is no commit yet for the files about to be committed...
I don't really see this being used by hand / outside of on a commit. But there is also no reason to complicate this more, and make it hard to call / deal with. The simple case should be simple. I can remove all commit-specific text from the error message, so this falls more logically to where you group mesos-style, but I think that just makes it harder to use / less obvious what the errors are without really gaining anything.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57081
-----------------------------------------------------------
On Oct. 21, 2014, 9:20 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 9:20 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Vinod Kone <vi...@gmail.com>.
> On Oct. 17, 2014, 12:54 a.m., Vinod Kone wrote:
> > support/mesos_split.py, line 27
> > <https://reviews.apache.org/r/26825/diff/1/?file=723339#file723339line27>
> >
> > It's weird that this script talks about commits but takes a list of files. Why not have it take it a commit sha and let it convert to a list of files?
>
> Cody Maloney wrote:
> The code is a lot simpler.
>
> To go from sha-1 back I would need to import subprocess, have it call git then parse the output. Having bash + xargs just take care of that is a lot simpler (And also matches the pre-commit style hook).
I agree about the simplicity. But the weird part for me is that it is violating abstractions. Somehow, the script knows that a list of files given to it belongs to a commit and thats what its error message says. Maybe the error meessage need not mention commit? But we still need to say it is the commit that is in violation so that we give a better error message to ppl using it. Hmm.
Alternatively, do you think this script would be used for any reason other than to examine a commit? If not, I think it should take a commit.
> On Oct. 17, 2014, 12:54 a.m., Vinod Kone wrote:
> > support/mesos_split.py, line 59
> > <https://reviews.apache.org/r/26825/diff/1/?file=723339#file723339line59>
> >
> > No need to pull this out into a constant. Just print the string here.
>
> Cody Maloney wrote:
> Theh indentation gets particularly odd because the lines in the error message shouldn't be indented, so in the middle of a python block you end up with text full unindented which is rather odd. Making a multi-line string constant is the standard python way.
i see. ok.
- Vinod
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57081
-----------------------------------------------------------
On Oct. 21, 2014, 9:20 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 9:20 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Cody Maloney <co...@mesosphere.io>.
> On Oct. 17, 2014, 12:54 a.m., Vinod Kone wrote:
> > support/mesos_split.py, line 27
> > <https://reviews.apache.org/r/26825/diff/1/?file=723339#file723339line27>
> >
> > It's weird that this script talks about commits but takes a list of files. Why not have it take it a commit sha and let it convert to a list of files?
The code is a lot simpler.
To go from sha-1 back I would need to import subprocess, have it call git then parse the output. Having bash + xargs just take care of that is a lot simpler (And also matches the pre-commit style hook).
> On Oct. 17, 2014, 12:54 a.m., Vinod Kone wrote:
> > support/mesos_split.py, line 59
> > <https://reviews.apache.org/r/26825/diff/1/?file=723339#file723339line59>
> >
> > No need to pull this out into a constant. Just print the string here.
Theh indentation gets particularly odd because the lines in the error message shouldn't be indented, so in the middle of a python block you end up with text full unindented which is rather odd. Making a multi-line string constant is the standard python way.
- Cody
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57081
-----------------------------------------------------------
On Oct. 21, 2014, 9:20 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 21, 2014, 9:20 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>
Re: Review Request 26825: MESOS-1712: Automate disallowing of commits
mixing mesos/libprocess/stout
Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26825/#review57081
-----------------------------------------------------------
support/hooks/pre-commit
<https://reviews.apache.org/r/26825/#comment97562>
Add a comment?
# Check that the commits are properly split between mesos, libprocess and stout.
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97563>
period at the end.
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97564>
s/libraray./library/
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97565>
It's weird that this script talks about commits but takes a list of files. Why not have it take it a commit sha and let it convert to a list of files?
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97566>
period.
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97567>
How about?
"Please use separate commits for mesos, libprocess and stout."
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97568>
s/#Find/# Find/
period at the end.
support/mesos_split.py
<https://reviews.apache.org/r/26825/#comment97569>
No need to pull this out into a constant. Just print the string here.
- Vinod Kone
On Oct. 16, 2014, 9:04 p.m., Cody Maloney wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26825/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2014, 9:04 p.m.)
>
>
> Review request for mesos, Adam B and Vinod Kone.
>
>
> Bugs: MESOS-1712 and MESOS-1932
> https://issues.apache.org/jira/browse/MESOS-1712
> https://issues.apache.org/jira/browse/MESOS-1932
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds a new script mesos_split.py which is run as a git precommit hook. It will error if a commit is made which spans across multiple of the mesos projects (mesos, stout, libprocess)
>
> Sample output:
> ```
> ERROR: Commit spanning multiple projects
>
> Mesos rules state that a commit should only touch one mesos project (stout,
> libprocess, mesos).
>
> Paths grouped by project:
> mesos:
> baz
> libprocess:
> 3rdparty/libprocess/asdf
> stout:
> 3rdparty/libprocess/3rdparty/stout/foobar
> ```
>
>
> Diffs
> -----
>
> support/hooks/pre-commit f6910f852a51d64a3441f9c23e70cafc6f7de741
> support/mesos_split.py PRE-CREATION
>
> Diff: https://reviews.apache.org/r/26825/diff/
>
>
> Testing
> -------
>
> Git added files in various groupings of the subprojects. Tried committing, resulting in the sample error message above.
>
>
> Thanks,
>
> Cody Maloney
>
>