You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Michael Park <mc...@gmail.com> on 2014/09/03 01:04:05 UTC
Review Request 25275: Introduce ClangFormat to Mesos.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/
-----------------------------------------------------------
Review request for mesos, Adam B, Niklas Nielsen, and Timothy Chen.
Repository: mesos-git
Description
-------
Introduce ClangFormat which will
1. reduce the amount of time we spend on formatting
2. keep a consistent style across codebase
3. reduce the amount of style-related issues that lengthen review cycles
Diffs
-----
.clang-format PRE-CREATION
.gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
docs/clang-format.md PRE-CREATION
support/clang-format-3.4 PRE-CREATION
support/clang-format-3.5 PRE-CREATION
support/clang-format-mesos PRE-CREATION
Diff: https://reviews.apache.org/r/25275/diff/
Testing
-------
Ran ClangFormat on the codebase.
Thanks,
Michael Park
Re: Review Request 25275: Introduce ClangFormat to Mesos.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 8:44 p.m.)
Review request for mesos, Adam B, Cody Maloney, Dominic Hamon, Niklas Nielsen, and Timothy Chen.
Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291
Repository: mesos-git
Description
-------
Introduce ClangFormat which will
1. reduce the amount of time we spend on formatting
2. keep a consistent style across codebase
3. reduce the amount of style-related issues that lengthen review cycles
Diffs
-----
.clang-format PRE-CREATION
.gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
docs/clang-format.md PRE-CREATION
support/clang-format-3.4 PRE-CREATION
support/clang-format-3.5 PRE-CREATION
support/clang-format-mesos PRE-CREATION
Diff: https://reviews.apache.org/r/25275/diff/
Testing
-------
Ran ClangFormat on the codebase.
Thanks,
Michael Park
Re: Review Request 25275: Introduce ClangFormat to Mesos.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 8:43 p.m.)
Review request for mesos, Adam B, Cody Maloney, Niklas Nielsen, and Timothy Chen.
Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291
Repository: mesos-git
Description
-------
Introduce ClangFormat which will
1. reduce the amount of time we spend on formatting
2. keep a consistent style across codebase
3. reduce the amount of style-related issues that lengthen review cycles
Diffs
-----
.clang-format PRE-CREATION
.gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
docs/clang-format.md PRE-CREATION
support/clang-format-3.4 PRE-CREATION
support/clang-format-3.5 PRE-CREATION
support/clang-format-mesos PRE-CREATION
Diff: https://reviews.apache.org/r/25275/diff/
Testing
-------
Ran ClangFormat on the codebase.
Thanks,
Michael Park
Re: Review Request 25275: Introduce ClangFormat to Mesos.
Posted by Michael Park <mc...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/
-----------------------------------------------------------
(Updated Sept. 2, 2014, 8:34 p.m.)
Review request for mesos, Adam B, Niklas Nielsen, and Timothy Chen.
Bugs: MESOS-1291
https://issues.apache.org/jira/browse/MESOS-1291
Repository: mesos-git
Description
-------
Introduce ClangFormat which will
1. reduce the amount of time we spend on formatting
2. keep a consistent style across codebase
3. reduce the amount of style-related issues that lengthen review cycles
Diffs
-----
.clang-format PRE-CREATION
.gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
docs/clang-format.md PRE-CREATION
support/clang-format-3.4 PRE-CREATION
support/clang-format-3.5 PRE-CREATION
support/clang-format-mesos PRE-CREATION
Diff: https://reviews.apache.org/r/25275/diff/
Testing
-------
Ran ClangFormat on the codebase.
Thanks,
Michael Park
Re: Review Request 25275: Introduce ClangFormat to Mesos.
Posted by Michael Park <mc...@gmail.com>.
> On Sept. 2, 2014, 7:22 p.m., Dominic Hamon wrote:
> > docs/clang-format.md, line 67
> > <https://reviews.apache.org/r/25275/diff/1/?file=674485#file674485line67>
> >
> > this is great! however, i think it's too much overhead to expect people to use it. for broad adoption we should stick to one version (probably 3.4) and update clang-format as new versions become available.
> >
> > i actually think we should hold off until 3.5 is more widely available so we get foreach wrapping correctly.
I agree that we should probably hold off till 3.5. It's scheduled to be released tomorrow but I don't think it'll be on schedule.
I really don't expect the patched version to be used by many (if any) people. But I'll be using the patched version myself so I wanted to share it in case the existing limitations bothered someone enough to want to use it.
> On Sept. 2, 2014, 7:22 p.m., Dominic Hamon wrote:
> > support/clang-format-3.5, line 2
> > <https://reviews.apache.org/r/25275/diff/1/?file=674487#file674487line2>
> >
> > I found BasedOnStyle: Chromium to give better results when I was playing with this. Did you see MESOS-1291?
Just saw MESOS-1291. There are 6 attributes that differ between Google and Chromium styles, 3 of them we want Google's and 3 of them we want Chromium's (I think). I chose Google style mostly because the way Mesos C++ Style Guide is outlined, where it basically says `Google` + a few differences.
> On Sept. 2, 2014, 7:22 p.m., Dominic Hamon wrote:
> > support/clang-format-mesos, line 56
> > <https://reviews.apache.org/r/25275/diff/1/?file=674488#file674488line56>
> >
> > the other option here is to persuade the powers that be that clang-format is the way forward and we should pick something close to the built-in that works with 3.5 (widely available?) or even 3.4 and go with that.
`clang-format-3.4` and `clang-format-3.5` both use `Linux` here which is the closest to Mesos style guide (I see you found the same in MESOS-1291). `clang-format-mesos` is an effort to get `clang-format` to do exactly what we want for `Mesos`. `BreakBeforeBraces: Mesos` in specific fixes:
1. braces after `namespace` should not be wrapped
2. braces after `struct` and `union` should be wrapped like `class`.
But again, `clang-format-mesos` is only for people who will bother going through the trouble to get more precise formatting from `clang-format`. I expect most people to just go with `clang-format-3.4` for now and `clang-format-3.5` once it's available.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/#review52102
-----------------------------------------------------------
On Sept. 2, 2014, 7:04 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25275/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 7:04 p.m.)
>
>
> Review request for mesos, Adam B, Niklas Nielsen, and Timothy Chen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce ClangFormat which will
> 1. reduce the amount of time we spend on formatting
> 2. keep a consistent style across codebase
> 3. reduce the amount of style-related issues that lengthen review cycles
>
>
> Diffs
> -----
>
> .clang-format PRE-CREATION
> .gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
> docs/clang-format.md PRE-CREATION
> support/clang-format-3.4 PRE-CREATION
> support/clang-format-3.5 PRE-CREATION
> support/clang-format-mesos PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25275/diff/
>
>
> Testing
> -------
>
> Ran ClangFormat on the codebase.
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 25275: Introduce ClangFormat to Mesos.
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25275/#review52102
-----------------------------------------------------------
Fantastic - I don't know if you saw my work on this but i linked the bug (MESOS-1291) so you can see what direction i was going in.
It's worth considering if this can be installed as a pre-commit hook.
docs/clang-format.md
<https://reviews.apache.org/r/25275/#comment90846>
this is great! however, i think it's too much overhead to expect people to use it. for broad adoption we should stick to one version (probably 3.4) and update clang-format as new versions become available.
i actually think we should hold off until 3.5 is more widely available so we get foreach wrapping correctly.
support/clang-format-3.5
<https://reviews.apache.org/r/25275/#comment90844>
I found BasedOnStyle: Chromium to give better results when I was playing with this. Did you see MESOS-1291?
support/clang-format-mesos
<https://reviews.apache.org/r/25275/#comment90845>
the other option here is to persuade the powers that be that clang-format is the way forward and we should pick something close to the built-in that works with 3.5 (widely available?) or even 3.4 and go with that.
- Dominic Hamon
On Sept. 2, 2014, 4:04 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25275/
> -----------------------------------------------------------
>
> (Updated Sept. 2, 2014, 4:04 p.m.)
>
>
> Review request for mesos, Adam B, Niklas Nielsen, and Timothy Chen.
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Introduce ClangFormat which will
> 1. reduce the amount of time we spend on formatting
> 2. keep a consistent style across codebase
> 3. reduce the amount of style-related issues that lengthen review cycles
>
>
> Diffs
> -----
>
> .clang-format PRE-CREATION
> .gitignore-template 3e2b11ca8a9f9e77f58718ae3425a983a24d5865
> docs/clang-format.md PRE-CREATION
> support/clang-format-3.4 PRE-CREATION
> support/clang-format-3.5 PRE-CREATION
> support/clang-format-mesos PRE-CREATION
>
> Diff: https://reviews.apache.org/r/25275/diff/
>
>
> Testing
> -------
>
> Ran ClangFormat on the codebase.
>
>
> Thanks,
>
> Michael Park
>
>