You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Benjamin Mahler <be...@gmail.com> on 2015/11/05 22:15:45 UTC

Re: Mesos Style Guideline Adjustments

This has come up in a couple of reviews, seems like we should add some soft
guidelines around how to format comments for readability.

In particular, the reason that we wrapped at 70 in the past was for
readability, so it would be great to continue doing so as a soft stylistic
rule. The other thing we've been doing for readability is reducing
"jaggedness" (variability in line lengths).

It would be great to establish these as soft rules and encourage new
contributors / committers to follow them. Compare these two comments in
Master::updateTask. The first one wraps at 70 and reduces jagedness, the
second wraps at 80 and is more jagged:

https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072

I can provide more examples to help clarify. If no one objects, I'll follow
up with an update to the style guide. Thoughts appreciated!

On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <be...@mesosphere.io> wrote:

> +1
> > On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
> >
> > +1
> >
> > 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> >
> >> +1
> >>
> >>
> >>
> >>
> >> Thanks, Michael!
> >>
> >>
> >>
> >> —
> >> Sent from my iPhone, which is not as good as you'd hope to fix trypos n
> >> abbrvtn.
> >>
> >> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com> wrote:
> >>
> >>> I've removed the 70 column restriction on comments from the style
> guide:
> >>>
> >>
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> >>> Also, based on the comments, it seems like we should allow 80 column
> >>> comments but omit the sweeping change.
> >>> Thanks,
> >>> MPark.
> >>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <ma...@mesosphere.io>
> >> wrote:
> >>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <be...@mesosphere.io>
> >>>> wrote:
> >>>>
> >>>>> Like BenM,
> >>>>>
> >>>>> +1 on allowing 80 column comments
> >>>>>
> >>>> +1
> >>>> (it really IS annoying having to keep an eye on the bottom column
> >> counter
> >>>> when typing comments :)
> >>>>
> >>>>
> >>>>> -1 on sweeping changes; incremental changes when touching old
> comments
> >>>>> will do IMHO
> >>>>>
> >>>>> +1 on the -1? :)
> >>>> Incremental changes are good and I doubt anyone will be "confused" by
> >> them.
> >>>>
> >>>>
> >>>>> Bernd
> >>>>>
> >>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mc...@gmail.com>
> >> wrote:
> >>>>>>
> >>>>>> Ben, thanks for your input!
> >>>>>>
> >>>>>> Another update on this topic: the patches around break before braces
> >>>> for
> >>>>>> *enum* style and overloaded operators have been committed.
> >>>>>>
> >>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> >>>>> benjamin.mahler@gmail.com>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> We already don't necessarily wrap at 70 characters (often we wrap
> >>>>> before 70
> >>>>>>> to reduce "jaggedness" or to make it look cleaner).
> >>>>>>>
> >>>>>>> So with the change to 80, this still makes all existing comments
> >>>> valid.
> >>>>> We
> >>>>>>> can still encourage folks to write paragraphs in a way that is
> >> easy to
> >>>>>>> digest for the reader. That is, I think we should still be trying
> >> not
> >>>> to
> >>>>>>> write jagged paragraphs of comments, it's just not a hard stylistic
> >>>>>>> violation given we don't have an algorithm for this.
> >>>>>>>
> >>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to sweeping
> >>>> across
> >>>>> all
> >>>>>>> the comments or doing wrapping based only on line length rather
> >> than
> >>>>>>> jaggedness going forward.
> >>>>>>>
> >>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> >>>>> joris@mesosphere.io>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> I will volunteer to update all the comments to wrap at 80 if we
> >> agree
> >>>>> to
> >>>>>>>> keep the code base consistent.
> >>>>>>>> Naturally that is also my vote ;-)
> >>>>>>>> Joris
> >>>>>>>>
> >>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <mc...@gmail.com>
> >> wrote:
> >>>>>>>>>
> >>>>>>>>> An update on this topic since we covered it at the community
> >>>> developer
> >>>>>>>> sync.
> >>>>>>>>>
> >>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as their
> >>>> style
> >>>>>>>> is
> >>>>>>>>> equivalent to ours. The only change this entails for our
> >> codebase
> >>>> is
> >>>>>>> to
> >>>>>>>>> consistently wrap the braces for *enum* definitions, as we're
> >>>>>>> currently
> >>>>>>>>> inconsistent. I've taken on the work involved in this change:
> >>>>>>>>>    - stout: https://reviews.apache.org/r/37258
> >>>>>>>>>    - libprocess: https://reviews.apache.org/r/37259
> >>>>>>>>>    - mesos: https://reviews.apache.org/r/37260
> >>>>>>>>>    2. We will drop the rule for adding spaces around overloaded
> >>>>>>>>> operators. We'll simply do a sweep of the codebase to update
> >> all of
> >>>>>>>> them
> >>>>>>>>> consistently. Artem has kindly taken action on this already:
> >>>>>>>>>    - stout: https://reviews.apache.org/r/37018/
> >>>>>>>>>    - libprocess: https://reviews.apache.org/r/37017/
> >>>>>>>>>    - mesos: https://reviews.apache.org/r/37013/
> >>>>>>>>>    3. We will drop the rule for wrapping comments at 70
> >> characters.
> >>>>>>> We
> >>>>>>>>> have a few options to proceed here:
> >>>>>>>>>    - Keep all the existing comments in tact, and simply allow
> >> new
> >>>>>>>>>    comments to wrap at 80, this is less work.
> >>>>>>>>>    - Update all instances of the comments wrapping at 70 to be
> >>>>>>> wrapped
> >>>>>>>>>    at 80, so that we can be consistent.
> >>>>>>>>>
> >>>>>>>>> I proposed that we simply allow new comments to wrap at 80, but I
> >>>> have
> >>>>>>>>> heard arguments to update the existing comments, so that we can
> >> be
> >>>>>>>>> consistent across the codebase. If you have a suggestion/opinion
> >> on
> >>>>> how
> >>>>>>>> we
> >>>>>>>>> should proceed with (3), please share!
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>>
> >>>>>>>>> MPark.
> >>>>>>>>>
> >>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> >>>>>>> alexander@mesosphere.io>
> >>>>>>>>> wrote:
> >>>>>>>>>
> >>>>>>>>>> I also vote up for that! I rather change our guidelines a little
> >>>> bit
> >>>>>>>> than
> >>>>>>>>>> waiting for months
> >>>>>>>>>> to get our changes into the clang-format source without any
> >>>> security
> >>>>>>>> that
> >>>>>>>>>> it will actually land.
> >>>>>>>>>>
> >>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <al...@mesosphere.com>
> >>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>> I think automation is very important. If we should slightly
> >> change
> >>>>>>> our
> >>>>>>>>>>> style in order to set-up easily enforceable rules, I vote with
> >>>> both
> >>>>>>>> hands
> >>>>>>>>>>> for that.
> >>>>>>>>>>>
> >>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> >> mcypark@gmail.com
> >>>>>
> >>>>>>>> wrote:
> >>>>>>>>>>>>
> >>>>>>>>>>>> Oops, sorry I was so excited that this could just solve the
> >> issue
> >>>>>>>> that I
> >>>>>>>>>>>> forgot to answer your question.
> >>>>>>>>>>>>
> >>>>>>>>>>>> In general, the clang-format strives to adopt widely used
> >> styles,
> >>>>>>>> which
> >>>>>>>>>> I'm
> >>>>>>>>>>>> not sure if we would be considered widely used. Aside from
> >> that,
> >>>>>>>> another
> >>>>>>>>>>>> concern was that it could take a while for our style
> >> proposals to
> >>>>>>> make
> >>>>>>>>>> it
> >>>>>>>>>>>> upstream and for it to be useful.
> >>>>>>>>>>>>
> >>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> >> mcypark@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Is it worth adding our own style?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> >> WebKit.).
> >>>>>>> How
> >>>>>>>>>>>>>> hard is it?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I was just looking into this again and *Mozilla* was added as
> >>>> the
> >>>>>>>>>> newest
> >>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on enum,
> >>>>>>> function,
> >>>>>>>>>> and
> >>>>>>>>>>>>> record definitions (struct, class, union). I think we can
> >>>> actually
> >>>>>>>> use
> >>>>>>>>>>>> that
> >>>>>>>>>>>>> one and be done with it. Having looked through the codebase,
> >> we
> >>>>>>> wrap
> >>>>>>>>>> the
> >>>>>>>>>>>>> braces for *enum* for about half of the cases. It would be
> >> about
> >>>>> 35
> >>>>>>>>>>>>> instances that we have to fix from what I can see in our
> >>>> codebase.
> >>>>>>>> What
> >>>>>>>>>>>> do
> >>>>>>>>>>>>> you think?
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> >>>>>>>>>>>> benjamin.mahler@gmail.com>
> >>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> Is it worth adding our own style?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> >>>> WebKit.).
> >>>>>>> How
> >>>>>>>>>>>> hard
> >>>>>>>>>>>>>> is it?
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> >>>> mcypark@gmail.com
> >>>>>>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines that I
> >>>> would
> >>>>>>>> like
> >>>>>>>>>>>> to
> >>>>>>>>>>>>>>> propose to drop. They are:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> >>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the brace
> >> after
> >>>>>>>>>>>>>>> *namespace*,
> >>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at all.
> >> But
> >>>> it
> >>>>>>>> is
> >>>>>>>>>>>>>>> consistent throughout the codebase.
> >>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
> >>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> The main motivation is that as a community we would like to
> >>>>>>> reduce
> >>>>>>>>>> the
> >>>>>>>>>>>>>>> discrepancy between what *clang-format* produces. This is a
> >>>> dual
> >>>>>>>>>>>>>> effort, as
> >>>>>>>>>>>>>>> we work on improving *clang-format* to support some of our
> >>>> style
> >>>>>>>>>> which
> >>>>>>>>>>>>>> is
> >>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the function
> >>>>>>>> arguments
> >>>>>>>>>>>> to
> >>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request which
> >> is
> >>>>>>> being
> >>>>>>>>>>>>>> tracked
> >>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> >> instances
> >>>> of
> >>>>>>>> (1)
> >>>>>>>>>>>> and
> >>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the constraint
> >> from
> >>>> 70
> >>>>>>> to
> >>>>>>>>>> 80
> >>>>>>>>>>>>>>> without touching the existing comments.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping any of
> >>>> the 3
> >>>>>>>>>> rules
> >>>>>>>>>>>>>>> above?
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> MPark.
> >>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>
> >>>>>
> >>>>
> >>
> >
> >
> >
> > --
> > Deshi Xiao
> > Twitter: xds2000
> > E-mail: xiaods(AT)gmail.com
>
>

Re: Mesos Style Guideline Adjustments

Posted by Benjamin Mahler <be...@gmail.com>.
+benh

I seem to recall benh having something in emacs that formatted his comments
nicely, which I think formed the basis for thinking about jaggedness and
wrapping at 70 vs 80.

On Fri, Nov 6, 2015 at 11:26 AM, Joris Van Remoortere <jo...@mesosphere.io>
wrote:

> @ben Is "jaggedness" the only *formatting* influence on the readability of
> the comments? If so, would it be possible to make a simple github.io page
> where we can paste comments, and they get formatted for minimal jaggedness
> based on some simple math? This would help provide consistency between
> contributions, and likely better results than humans trying to optimize the
> jaggedness equation.
>
> This way we can focus on the content of the comments when reviewing, rather
> than the format. Later one we might even be able to incorporate this in
> more intelligent editor friendly formatting tools.
>
> If there is more than some simple math involved, this may not be a viable
> solution.
>
> Joris
>
> —
> *Joris Van Remoortere*
> Mesosphere
>
> On Fri, Nov 6, 2015 at 11:10 AM, Benjamin Mahler <
> benjamin.mahler@gmail.com>
> wrote:
>
> > I raise this because there is a ton of value in keeping our codebase as
> > readable as possible. Having had to navigate and maintain the code base
> for
> > the past few years, this _is_ a "real issue" that deserves the
> contributors
> > spending their mental resources on. I realize this seems very subtle, but
> > it is of critical importance for the maintainability of the project that
> > folks are paying attention to how their code and comments will be read by
> > others.
> >
> > I think about this as follows: we'll always have "soft" rules that cannot
> > be enforced by these style checkers but require mentorship to communicate
> > as we grow the community. These tools cannot tell you how to structure
> your
> > code in a simple form. They also can't tell you how to write a comment
> in a
> > way that is clear to others.
> >
> > To Alex's example, two comments:
> >
> > (1) The second comment is poorly written, did no one even notice this??
> > That's a "real issue" :(
> > (2) It's important not to isolate the comments from the code. The
> comments
> > live with the code and a major reason why long line length paragraphs are
> > irregular is because the majority of code lines do not approach 80
> > characters.
> > (3) We tend to separate "multiple logical parts" of a comment with an
> empty
> > // line, which helps readability.
> > (4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
> > "jaggedness" (or to increase "regularity") and (b) long line lengths (80)
> > for large paragraphs are harder to read.
> >
> > I just don't want the formatter to become a crutch that causes folks to
> > think less about the implications of how they write their comments. Do
> the
> > soft rules in (a) and (b) seem reasonable? We already need to be diligent
> > in reviews to ensure that comments are well-written.
> >
> > On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
> > benjamin.bannier@mesosphere.io> wrote:
> >
> > > Hi,
> > >
> > > just to echo Alexander’s point, for newbies like me being able to
> > delegate
> > > formatting decisions to tools as much as possible frees up a lot of
> > mental
> > > resources for tackling the real issues.
> > >
> > >
> > > Cheers,
> > >
> > > Benjamin
> > >
> > > ps. Also looking forward to an updated and expanded clang-format
> config.
> > >
> > >
> > > > On Nov 6, 2015, at 1:44 PM, Alexander Rojas <alexander@mesosphere.io
> >
> > > wrote:
> > > >
> > > > I think one of the main reasons we move to having 80 as the limit for
> > > both code and comments is the ability it gives us to use tools (e.g.
> > > clang-format) to enforce formatting rules, so personally I rather have
> us
> > > putting effort towards that goal. On that note, the developer branch of
> > > clang-format allows a much closer formatting options to the ones we
> use.
> > On
> > > OS X it can be installed using `brew install --HEAD clang-format`.
> > > >
> > > > Right now I’m working on setting the config file to be as close as
> > > possible to our style.
> > > >
> > > >> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com>
> wrote:
> > > >>
> > > >> I think jaggedness in the example you provide comes mainly from the
> > fact
> > > >> that the second comment has multiple logical blocks. I have
> formatted
> > > both
> > > >> comments at 70 and at 80, here is the outcome:
> > > http://pastebin.com/nRQB0nCD
> > > >>
> > > >> While the first comment indeed looks better when wrapped at 70, I
> > can't
> > > say
> > > >> the same about the second one.
> > > >>
> > > >> I would say, that the longer a line could be, the less jagged the
> > > comment
> > > >> block is. The ratio (`averageWordLength` / `maxLineLength`)
> approaches
> > > 0 as
> > > >> `maxLineLenght` approaches infinity, which means wrapping a long
> word
> > > right
> > > >> before the line end should be perceived less jagged : ).
> > > >>
> > > >> Also, the longer an individual line can be, the less total lines are
> > > needed
> > > >> for a comment block, which reduces jaggedness and makes code a
> little
> > > bit
> > > >> more readable.
> > > >>
> > > >> But my strongest argument is that having a separate soft rule for
> > > comments
> > > >> is hard to enforce. I think what we can do is to encourage
> > contributors
> > > /
> > > >> committers to wrap comments in the most logical way—like the first
> > > comment
> > > >> in the example you provide—even if the line length is not fully
> > > utilized.
> > > >> Having said that, I would rather keep a single number: hard limit at
> > 80
> > > for
> > > >> simplicity.
> > > >>
> > > >>
> > > >>
> > > >> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
> > > benjamin.mahler@gmail.com>
> > > >> wrote:
> > > >>
> > > >>> This has come up in a couple of reviews, seems like we should add
> > some
> > > soft
> > > >>> guidelines around how to format comments for readability.
> > > >>>
> > > >>> In particular, the reason that we wrapped at 70 in the past was for
> > > >>> readability, so it would be great to continue doing so as a soft
> > > stylistic
> > > >>> rule. The other thing we've been doing for readability is reducing
> > > >>> "jaggedness" (variability in line lengths).
> > > >>>
> > > >>> It would be great to establish these as soft rules and encourage
> new
> > > >>> contributors / committers to follow them. Compare these two
> comments
> > in
> > > >>> Master::updateTask. The first one wraps at 70 and reduces
> jagedness,
> > > the
> > > >>> second wraps at 80 and is more jagged:
> > > >>>
> > > >>>
> > >
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
> > > >>>
> > >
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
> > > >>>
> > > >>> I can provide more examples to help clarify. If no one objects,
> I'll
> > > follow
> > > >>> up with an update to the style guide. Thoughts appreciated!
> > > >>>
> > > >>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <
> bernd@mesosphere.io
> > >
> > > >>> wrote:
> > > >>>
> > > >>>> +1
> > > >>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com>
> wrote:
> > > >>>>>
> > > >>>>> +1
> > > >>>>>
> > > >>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> > > >>>>>
> > > >>>>>> +1
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> Thanks, Michael!
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> —
> > > >>>>>> Sent from my iPhone, which is not as good as you'd hope to fix
> > > trypos
> > > >>> n
> > > >>>>>> abbrvtn.
> > > >>>>>>
> > > >>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mcypark@gmail.com
> >
> > > >>> wrote:
> > > >>>>>>
> > > >>>>>>> I've removed the 70 column restriction on comments from the
> style
> > > >>>> guide:
> > > >>>>>>>
> > > >>>>>>
> > > >>>>
> > > >>>
> > >
> >
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> > > >>>>>>> Also, based on the comments, it seems like we should allow 80
> > > column
> > > >>>>>>> comments but omit the sweeping change.
> > > >>>>>>> Thanks,
> > > >>>>>>> MPark.
> > > >>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
> > > marco@mesosphere.io
> > > >>>>
> > > >>>>>> wrote:
> > > >>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
> > > >>> bernd@mesosphere.io>
> > > >>>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Like BenM,
> > > >>>>>>>>>
> > > >>>>>>>>> +1 on allowing 80 column comments
> > > >>>>>>>>>
> > > >>>>>>>> +1
> > > >>>>>>>> (it really IS annoying having to keep an eye on the bottom
> > column
> > > >>>>>> counter
> > > >>>>>>>> when typing comments :)
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>> -1 on sweeping changes; incremental changes when touching old
> > > >>>> comments
> > > >>>>>>>>> will do IMHO
> > > >>>>>>>>>
> > > >>>>>>>>> +1 on the -1? :)
> > > >>>>>>>> Incremental changes are good and I doubt anyone will be
> > "confused"
> > > >>> by
> > > >>>>>> them.
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>> Bernd
> > > >>>>>>>>>
> > > >>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <
> mcypark@gmail.com
> > >
> > > >>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>> Ben, thanks for your input!
> > > >>>>>>>>>>
> > > >>>>>>>>>> Another update on this topic: the patches around break
> before
> > > >>> braces
> > > >>>>>>>> for
> > > >>>>>>>>>> *enum* style and overloaded operators have been committed.
> > > >>>>>>>>>>
> > > >>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> > > >>>>>>>>> benjamin.mahler@gmail.com>
> > > >>>>>>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>>> We already don't necessarily wrap at 70 characters (often
> we
> > > wrap
> > > >>>>>>>>> before 70
> > > >>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> So with the change to 80, this still makes all existing
> > > comments
> > > >>>>>>>> valid.
> > > >>>>>>>>> We
> > > >>>>>>>>>>> can still encourage folks to write paragraphs in a way that
> > is
> > > >>>>>> easy to
> > > >>>>>>>>>>> digest for the reader. That is, I think we should still be
> > > trying
> > > >>>>>> not
> > > >>>>>>>> to
> > > >>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
> > > >>> stylistic
> > > >>>>>>>>>>> violation given we don't have an algorithm for this.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to
> > > sweeping
> > > >>>>>>>> across
> > > >>>>>>>>> all
> > > >>>>>>>>>>> the comments or doing wrapping based only on line length
> > rather
> > > >>>>>> than
> > > >>>>>>>>>>> jaggedness going forward.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> > > >>>>>>>>> joris@mesosphere.io>
> > > >>>>>>>>>>> wrote:
> > > >>>>>>>>>>>
> > > >>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80
> if
> > > we
> > > >>>>>> agree
> > > >>>>>>>>> to
> > > >>>>>>>>>>>> keep the code base consistent.
> > > >>>>>>>>>>>> Naturally that is also my vote ;-)
> > > >>>>>>>>>>>> Joris
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <
> > mcypark@gmail.com>
> > > >>>>>> wrote:
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> An update on this topic since we covered it at the
> > community
> > > >>>>>>>> developer
> > > >>>>>>>>>>>> sync.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
> > > their
> > > >>>>>>>> style
> > > >>>>>>>>>>>> is
> > > >>>>>>>>>>>>> equivalent to ours. The only change this entails for our
> > > >>>>>> codebase
> > > >>>>>>>> is
> > > >>>>>>>>>>> to
> > > >>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as
> > we're
> > > >>>>>>>>>>> currently
> > > >>>>>>>>>>>>> inconsistent. I've taken on the work involved in this
> > change:
> > > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37258
> > > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37259
> > > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37260
> > > >>>>>>>>>>>>>  2. We will drop the rule for adding spaces around
> > overloaded
> > > >>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to
> > update
> > > >>>>>> all of
> > > >>>>>>>>>>>> them
> > > >>>>>>>>>>>>> consistently. Artem has kindly taken action on this
> > already:
> > > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37018/
> > > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37017/
> > > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37013/
> > > >>>>>>>>>>>>>  3. We will drop the rule for wrapping comments at 70
> > > >>>>>> characters.
> > > >>>>>>>>>>> We
> > > >>>>>>>>>>>>> have a few options to proceed here:
> > > >>>>>>>>>>>>>  - Keep all the existing comments in tact, and simply
> allow
> > > >>>>>> new
> > > >>>>>>>>>>>>>  comments to wrap at 80, this is less work.
> > > >>>>>>>>>>>>>  - Update all instances of the comments wrapping at 70 to
> > be
> > > >>>>>>>>>>> wrapped
> > > >>>>>>>>>>>>>  at 80, so that we can be consistent.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at
> 80,
> > > >>> but I
> > > >>>>>>>> have
> > > >>>>>>>>>>>>> heard arguments to update the existing comments, so that
> we
> > > can
> > > >>>>>> be
> > > >>>>>>>>>>>>> consistent across the codebase. If you have a
> > > >>> suggestion/opinion
> > > >>>>>> on
> > > >>>>>>>>> how
> > > >>>>>>>>>>>> we
> > > >>>>>>>>>>>>> should proceed with (3), please share!
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> Thanks,
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> MPark.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> > > >>>>>>>>>>> alexander@mesosphere.io>
> > > >>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines
> a
> > > >>> little
> > > >>>>>>>> bit
> > > >>>>>>>>>>>> than
> > > >>>>>>>>>>>>>> waiting for months
> > > >>>>>>>>>>>>>> to get our changes into the clang-format source without
> > any
> > > >>>>>>>> security
> > > >>>>>>>>>>>> that
> > > >>>>>>>>>>>>>> it will actually land.
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
> > > >>> alex@mesosphere.com>
> > > >>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> I think automation is very important. If we should
> > slightly
> > > >>>>>> change
> > > >>>>>>>>>>> our
> > > >>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I
> vote
> > > >>> with
> > > >>>>>>>> both
> > > >>>>>>>>>>>> hands
> > > >>>>>>>>>>>>>>> for that.
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> > > >>>>>> mcypark@gmail.com
> > > >>>>>>>>>
> > > >>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just
> solve
> > > the
> > > >>>>>> issue
> > > >>>>>>>>>>>> that I
> > > >>>>>>>>>>>>>>>> forgot to answer your question.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely
> > used
> > > >>>>>> styles,
> > > >>>>>>>>>>>> which
> > > >>>>>>>>>>>>>> I'm
> > > >>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside
> > from
> > > >>>>>> that,
> > > >>>>>>>>>>>> another
> > > >>>>>>>>>>>>>>>> concern was that it could take a while for our style
> > > >>>>>> proposals to
> > > >>>>>>>>>>> make
> > > >>>>>>>>>>>>>> it
> > > >>>>>>>>>>>>>>>> upstream and for it to be useful.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> > > >>>>>> mcypark@gmail.com>
> > > >>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
> Mozilla,
> > > >>>>>> WebKit.).
> > > >>>>>>>>>>> How
> > > >>>>>>>>>>>>>>>>>> hard is it?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was
> > > added
> > > >>> as
> > > >>>>>>>> the
> > > >>>>>>>>>>>>>> newest
> > > >>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on
> > > enum,
> > > >>>>>>>>>>> function,
> > > >>>>>>>>>>>>>> and
> > > >>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we
> > can
> > > >>>>>>>> actually
> > > >>>>>>>>>>>> use
> > > >>>>>>>>>>>>>>>> that
> > > >>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
> > > >>> codebase,
> > > >>>>>> we
> > > >>>>>>>>>>> wrap
> > > >>>>>>>>>>>>>> the
> > > >>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It
> would
> > > be
> > > >>>>>> about
> > > >>>>>>>>> 35
> > > >>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in
> > our
> > > >>>>>>>> codebase.
> > > >>>>>>>>>>>> What
> > > >>>>>>>>>>>>>>>> do
> > > >>>>>>>>>>>>>>>>> you think?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> > > >>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
> > > >>>>>>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
> Mozilla,
> > > >>>>>>>> WebKit.).
> > > >>>>>>>>>>> How
> > > >>>>>>>>>>>>>>>> hard
> > > >>>>>>>>>>>>>>>>>> is it?
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> > > >>>>>>>> mcypark@gmail.com
> > > >>>>>>>>>>
> > > >>>>>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines
> > > that I
> > > >>>>>>>> would
> > > >>>>>>>>>>>> like
> > > >>>>>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>>>>> propose to drop. They are:
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> > > >>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the
> > brace
> > > >>>>>> after
> > > >>>>>>>>>>>>>>>>>>> *namespace*,
> > > >>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule
> at
> > > all.
> > > >>>>>> But
> > > >>>>>>>> it
> > > >>>>>>>>>>>> is
> > > >>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
> > > >>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with
> spaces.
> > > >>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would
> > > like
> > > >>> to
> > > >>>>>>>>>>> reduce
> > > >>>>>>>>>>>>>> the
> > > >>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces.
> > This
> > > >>> is a
> > > >>>>>>>> dual
> > > >>>>>>>>>>>>>>>>>> effort, as
> > > >>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some
> > of
> > > >>> our
> > > >>>>>>>> style
> > > >>>>>>>>>>>>>> which
> > > >>>>>>>>>>>>>>>>>> is
> > > >>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
> > > >>> function
> > > >>>>>>>>>>>> arguments
> > > >>>>>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request
> > > which
> > > >>>>>> is
> > > >>>>>>>>>>> being
> > > >>>>>>>>>>>>>>>>>> tracked
> > > >>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> > > >>>>>> instances
> > > >>>>>>>> of
> > > >>>>>>>>>>>> (1)
> > > >>>>>>>>>>>>>>>> and
> > > >>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the
> > > constraint
> > > >>>>>> from
> > > >>>>>>>> 70
> > > >>>>>>>>>>> to
> > > >>>>>>>>>>>>>> 80
> > > >>>>>>>>>>>>>>>>>>> without touching the existing comments.
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping
> > any
> > > >>> of
> > > >>>>>>>> the 3
> > > >>>>>>>>>>>>>> rules
> > > >>>>>>>>>>>>>>>>>>> above?
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Thanks,
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> MPark.
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> --
> > > >>>>> Deshi Xiao
> > > >>>>> Twitter: xds2000
> > > >>>>> E-mail: xiaods(AT)gmail.com
> > > >>>>
> > > >>>>
> > > >>>
> > > >
> > >
> > >
> >
>

Re: Mesos Style Guideline Adjustments

Posted by Benjamin Mahler <be...@gmail.com>.
We can't only look at comment formatting in a vacuum, since our comments
live with the code. For example, when you look at the following two
comments along with the surrounding code:

(1)
https://github.com/apache/mesos/blob/0.26.0-rc1/src/master/allocator/mesos/hierarchical.cpp#L364
(2)
https://github.com/apache/mesos/blob/0.26.0-rc1/src/master/allocator/mesos/hierarchical.cpp#L574

I find that the longer lines in (2) make this less "regular". In other
words, code tends not to wrap at 80, so wrapping comments at 80 tends to
produce jaggedness between code and comments. That's essentially what I'm
finding unfortunate, and why it's important to look at comments alongside
code.

But at a higher level, we've introduced inconsistency which I regret. Now
we have comments at 70 and 80 which leads to even less readability. That's
another reason I wish we had stuck with 70: keep things consistent.

Going forward, it just seems messy to start wrapping at 80 when we have
such a vast amount of comments wrapped at 70. Re-writing all the existing
comments to wrap at 80 was covered in the proposal but is of course quite
disruptive.

On Mon, Nov 9, 2015 at 8:36 AM, Bernd Mathiske <be...@mesosphere.io> wrote:

> IMHO jaggedness is a secondary concern.
> I’d rather have subsentences on the same line
> than try to meet some page boundary on the right consistently.
> I see no value in that.
>
> Of course, a maximum line width of 80 columns is necessary
> due to anachronistic limitations of our tools.
> Here is the above paragraph without jaggedness, with 80 columns
> (which the email system will probably mess up before you get to see it):
>
> IMHO jaggedness is a secondary concern. I’d rather have subsentences on the
> same line  than try to meet some page boundary on the right consistently.
> I
> see no value in that.
>
> Here it is with 70 columns:
>
> IMHO jaggedness is a secondary concern. I’d rather have subsentences
> on the same line than try to meet some page boundary on the right
> consistently. I see no value in that.
>
> Maybe for some of us, avoiding jaggedness increases readability.
> For me personally, it decreases it.
>
> Bernd
>
> On Nov 6, 2015, at 8:36 PM, Kapil Arya <ka...@mesosphere.io> wrote:
>
> As far as setting a soft limit goes, I think there are several different
> style guides and it basically boils down to personal preferences as well.
> The Linux `fmt` command wraps by 75 columns by default; others use 72 or as
> extreme as 60 columns.
>
> Having said that, it's nice to have tools that enforce some
> check. Jaggedness isn't pretty and one can always use their judgement to
> reformat the comments to make it look nicer and more readable.
>
> My 2 cents.
>
> On Fri, Nov 6, 2015 at 2:26 PM, Joris Van Remoortere <jo...@mesosphere.io>
> wrote:
>
> @ben Is "jaggedness" the only *formatting* influence on the readability of
> the comments? If so, would it be possible to make a simple github.io page
> where we can paste comments, and they get formatted for minimal jaggedness
> based on some simple math? This would help provide consistency between
> contributions, and likely better results than humans trying to optimize the
> jaggedness equation.
>
> This way we can focus on the content of the comments when reviewing, rather
> than the format. Later one we might even be able to incorporate this in
> more intelligent editor friendly formatting tools.
>
> If there is more than some simple math involved, this may not be a viable
> solution.
>
> Joris
>
> —
> *Joris Van Remoortere*
> Mesosphere
>
> On Fri, Nov 6, 2015 at 11:10 AM, Benjamin Mahler <
> benjamin.mahler@gmail.com>
> wrote:
>
> I raise this because there is a ton of value in keeping our codebase as
> readable as possible. Having had to navigate and maintain the code base
>
> for
>
> the past few years, this _is_ a "real issue" that deserves the
>
> contributors
>
> spending their mental resources on. I realize this seems very subtle, but
> it is of critical importance for the maintainability of the project that
> folks are paying attention to how their code and comments will be read by
> others.
>
> I think about this as follows: we'll always have "soft" rules that cannot
> be enforced by these style checkers but require mentorship to communicate
> as we grow the community. These tools cannot tell you how to structure
>
> your
>
> code in a simple form. They also can't tell you how to write a comment
>
> in a
>
> way that is clear to others.
>
> To Alex's example, two comments:
>
> (1) The second comment is poorly written, did no one even notice this??
> That's a "real issue" :(
> (2) It's important not to isolate the comments from the code. The
>
> comments
>
> live with the code and a major reason why long line length paragraphs are
> irregular is because the majority of code lines do not approach 80
> characters.
> (3) We tend to separate "multiple logical parts" of a comment with an
>
> empty
>
> // line, which helps readability.
> (4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
> "jaggedness" (or to increase "regularity") and (b) long line lengths (80)
> for large paragraphs are harder to read.
>
> I just don't want the formatter to become a crutch that causes folks to
> think less about the implications of how they write their comments. Do
>
> the
>
> soft rules in (a) and (b) seem reasonable? We already need to be diligent
> in reviews to ensure that comments are well-written.
>
> On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
> benjamin.bannier@mesosphere.io> wrote:
>
> Hi,
>
> just to echo Alexander’s point, for newbies like me being able to
>
> delegate
>
> formatting decisions to tools as much as possible frees up a lot of
>
> mental
>
> resources for tackling the real issues.
>
>
> Cheers,
>
> Benjamin
>
> ps. Also looking forward to an updated and expanded clang-format
>
> config.
>
>
>
> On Nov 6, 2015, at 1:44 PM, Alexander Rojas <alexander@mesosphere.io
>
>
> wrote:
>
>
> I think one of the main reasons we move to having 80 as the limit for
>
> both code and comments is the ability it gives us to use tools (e.g.
> clang-format) to enforce formatting rules, so personally I rather have
>
> us
>
> putting effort towards that goal. On that note, the developer branch of
> clang-format allows a much closer formatting options to the ones we
>
> use.
>
> On
>
> OS X it can be installed using `brew install --HEAD clang-format`.
>
>
> Right now I’m working on setting the config file to be as close as
>
> possible to our style.
>
>
> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com>
>
> wrote:
>
>
> I think jaggedness in the example you provide comes mainly from the
>
> fact
>
> that the second comment has multiple logical blocks. I have
>
> formatted
>
> both
>
> comments at 70 and at 80, here is the outcome:
>
> http://pastebin.com/nRQB0nCD
>
>
> While the first comment indeed looks better when wrapped at 70, I
>
> can't
>
> say
>
> the same about the second one.
>
> I would say, that the longer a line could be, the less jagged the
>
> comment
>
> block is. The ratio (`averageWordLength` / `maxLineLength`)
>
> approaches
>
> 0 as
>
> `maxLineLenght` approaches infinity, which means wrapping a long
>
> word
>
> right
>
> before the line end should be perceived less jagged : ).
>
> Also, the longer an individual line can be, the less total lines are
>
> needed
>
> for a comment block, which reduces jaggedness and makes code a
>
> little
>
> bit
>
> more readable.
>
> But my strongest argument is that having a separate soft rule for
>
> comments
>
> is hard to enforce. I think what we can do is to encourage
>
> contributors
>
> /
>
> committers to wrap comments in the most logical way—like the first
>
> comment
>
> in the example you provide—even if the line length is not fully
>
> utilized.
>
> Having said that, I would rather keep a single number: hard limit at
>
> 80
>
> for
>
> simplicity.
>
>
>
> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
>
> benjamin.mahler@gmail.com>
>
> wrote:
>
> This has come up in a couple of reviews, seems like we should add
>
> some
>
> soft
>
> guidelines around how to format comments for readability.
>
> In particular, the reason that we wrapped at 70 in the past was for
> readability, so it would be great to continue doing so as a soft
>
> stylistic
>
> rule. The other thing we've been doing for readability is reducing
> "jaggedness" (variability in line lengths).
>
> It would be great to establish these as soft rules and encourage
>
> new
>
> contributors / committers to follow them. Compare these two
>
> comments
>
> in
>
> Master::updateTask. The first one wraps at 70 and reduces
>
> jagedness,
>
> the
>
> second wraps at 80 and is more jagged:
>
>
>
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
>
>
>
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>
>
> I can provide more examples to help clarify. If no one objects,
>
> I'll
>
> follow
>
> up with an update to the style guide. Thoughts appreciated!
>
> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <
>
> bernd@mesosphere.io
>
>
> wrote:
>
> +1
>
> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com>
>
> wrote:
>
>
> +1
>
> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
>
> +1
>
>
>
>
> Thanks, Michael!
>
>
>
> —
> Sent from my iPhone, which is not as good as you'd hope to fix
>
> trypos
>
> n
>
> abbrvtn.
>
> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mcypark@gmail.com
>
>
> wrote:
>
>
> I've removed the 70 column restriction on comments from the
>
> style
>
> guide:
>
>
>
>
>
>
>
>
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
>
> Also, based on the comments, it seems like we should allow 80
>
> column
>
> comments but omit the sweeping change.
> Thanks,
> MPark.
> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
>
> marco@mesosphere.io
>
>
> wrote:
>
> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
>
> bernd@mesosphere.io>
>
> wrote:
>
> Like BenM,
>
> +1 on allowing 80 column comments
>
> +1
> (it really IS annoying having to keep an eye on the bottom
>
> column
>
> counter
>
> when typing comments :)
>
>
> -1 on sweeping changes; incremental changes when touching old
>
> comments
>
> will do IMHO
>
> +1 on the -1? :)
>
> Incremental changes are good and I doubt anyone will be
>
> "confused"
>
> by
>
> them.
>
>
>
> Bernd
>
> On Aug 12, 2015, at 12:51 AM, Michael Park <
>
> mcypark@gmail.com
>
>
> wrote:
>
>
> Ben, thanks for your input!
>
> Another update on this topic: the patches around break
>
> before
>
> braces
>
> for
>
> *enum* style and overloaded operators have been committed.
>
> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
>
> benjamin.mahler@gmail.com>
>
> wrote:
>
> We already don't necessarily wrap at 70 characters (often
>
> we
>
> wrap
>
> before 70
>
> to reduce "jaggedness" or to make it look cleaner).
>
> So with the change to 80, this still makes all existing
>
> comments
>
> valid.
>
> We
>
> can still encourage folks to write paragraphs in a way that
>
> is
>
> easy to
>
> digest for the reader. That is, I think we should still be
>
> trying
>
> not
>
> to
>
> write jagged paragraphs of comments, it's just not a hard
>
> stylistic
>
> violation given we don't have an algorithm for this.
>
> So +1 to relaxing the hard 70 character rule, but -1 to
>
> sweeping
>
> across
>
> all
>
> the comments or doing wrapping based only on line length
>
> rather
>
> than
>
> jaggedness going forward.
>
> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
>
> joris@mesosphere.io>
>
> wrote:
>
> I will volunteer to update all the comments to wrap at 80
>
> if
>
> we
>
> agree
>
> to
>
> keep the code base consistent.
> Naturally that is also my vote ;-)
> Joris
>
> On Aug 8, 2015, at 1:40 PM, Michael Park <
>
> mcypark@gmail.com>
>
> wrote:
>
>
> An update on this topic since we covered it at the
>
> community
>
> developer
>
> sync.
>
>
> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
>
> their
>
> style
>
> is
>
> equivalent to ours. The only change this entails for our
>
> codebase
>
> is
>
> to
>
> consistently wrap the braces for *enum* definitions, as
>
> we're
>
> currently
>
> inconsistent. I've taken on the work involved in this
>
> change:
>
> - stout: https://reviews.apache.org/r/37258
> - libprocess: https://reviews.apache.org/r/37259
> - mesos: https://reviews.apache.org/r/37260
> 2. We will drop the rule for adding spaces around
>
> overloaded
>
> operators. We'll simply do a sweep of the codebase to
>
> update
>
> all of
>
> them
>
> consistently. Artem has kindly taken action on this
>
> already:
>
> - stout: https://reviews.apache.org/r/37018/
> - libprocess: https://reviews.apache.org/r/37017/
> - mesos: https://reviews.apache.org/r/37013/
> 3. We will drop the rule for wrapping comments at 70
>
> characters.
>
> We
>
> have a few options to proceed here:
> - Keep all the existing comments in tact, and simply
>
> allow
>
> new
>
> comments to wrap at 80, this is less work.
> - Update all instances of the comments wrapping at 70 to
>
> be
>
> wrapped
>
> at 80, so that we can be consistent.
>
> I proposed that we simply allow new comments to wrap at
>
> 80,
>
> but I
>
> have
>
> heard arguments to update the existing comments, so that
>
> we
>
> can
>
> be
>
> consistent across the codebase. If you have a
>
> suggestion/opinion
>
> on
>
> how
>
> we
>
> should proceed with (3), please share!
>
> Thanks,
>
> MPark.
>
> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
>
> alexander@mesosphere.io>
>
> wrote:
>
> I also vote up for that! I rather change our guidelines
>
> a
>
> little
>
> bit
>
> than
>
> waiting for months
> to get our changes into the clang-format source without
>
> any
>
> security
>
> that
>
> it will actually land.
>
> On 31 Jul 2015, at 09:31, Alex Rukletsov <
>
> alex@mesosphere.com>
>
> wrote:
>
>
> I think automation is very important. If we should
>
> slightly
>
> change
>
> our
>
> style in order to set-up easily enforceable rules, I
>
> vote
>
> with
>
> both
>
> hands
>
> for that.
>
> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
>
> mcypark@gmail.com
>
>
> wrote:
>
>
> Oops, sorry I was so excited that this could just
>
> solve
>
> the
>
> issue
>
> that I
>
> forgot to answer your question.
>
> In general, the clang-format strives to adopt widely
>
> used
>
> styles,
>
> which
>
> I'm
>
> not sure if we would be considered widely used. Aside
>
> from
>
> that,
>
> another
>
> concern was that it could take a while for our style
>
> proposals to
>
> make
>
> it
>
> upstream and for it to be useful.
>
> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
>
> mcypark@gmail.com>
>
> wrote:
>
>
> Is it worth adding our own style?
>
>
>
> I noticed other have (LLVM, Google, Chromium,
>
> Mozilla,
>
> WebKit.).
>
> How
>
> hard is it?
>
>
>
> I was just looking into this again and *Mozilla* was
>
> added
>
> as
>
> the
>
> newest
>
> *BreakBeforeBraces* style. It breaks before braces on
>
> enum,
>
> function,
>
> and
>
> record definitions (struct, class, union). I think we
>
> can
>
> actually
>
> use
>
> that
>
> one and be done with it. Having looked through the
>
> codebase,
>
> we
>
> wrap
>
> the
>
> braces for *enum* for about half of the cases. It
>
> would
>
> be
>
> about
>
> 35
>
> instances that we have to fix from what I can see in
>
> our
>
> codebase.
>
> What
>
> do
>
> you think?
>
> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
>
> benjamin.mahler@gmail.com>
>
> wrote:
>
> Is it worth adding our own style?
>
> I noticed other have (LLVM, Google, Chromium,
>
> Mozilla,
>
> WebKit.).
>
> How
>
> hard
>
> is it?
>
> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
>
> mcypark@gmail.com
>
>
> wrote:
>
>
> There are a few syntactical Mesos style guidelines
>
> that I
>
> would
>
> like
>
> to
>
> propose to drop. They are:
>
> 1. Open braces for namespace should not be wrapped.
> *NOTE*: The Google style guide does not wrap the
>
> brace
>
> after
>
> *namespace*,
> and the Mesos style guide does not mention a rule
>
> at
>
> all.
>
> But
>
> it
>
> is
>
> consistent throughout the codebase.
> 2. Overloaded operators should be padded with
>
> spaces.
>
> 3. Comments should be wrapped at 70 characters.
>
> The main motivation is that as a community we would
>
> like
>
> to
>
> reduce
>
> the
>
> discrepancy between what *clang-format* produces.
>
> This
>
> is a
>
> dual
>
> effort, as
>
> we work on improving *clang-format* to support some
>
> of
>
> our
>
> style
>
> which
>
> is
>
> popular in the C++ community as well. Wrapping the
>
> function
>
> arguments
>
> to
>
> avoid "jaggedness" for example is a feature request
>
> which
>
> is
>
> being
>
> tracked
>
> at: https://llvm.org/bugs/show_bug.cgi?id=23422
>
> Going forward, the proposal is to update all of the
>
> instances
>
> of
>
> (1)
>
> and
>
> (2) at once. For (3), we can simply relax the
>
> constraint
>
> from
>
> 70
>
> to
>
> 80
>
> without touching the existing comments.
>
> Does anyone have any strong opinions about dropping
>
> any
>
> of
>
> the 3
>
> rules
>
> above?
>
> Thanks,
>
> MPark.
>
>
>
>
>
>
>
>
>
>
>
>
> --
> Deshi Xiao
> Twitter: xds2000
> E-mail: xiaods(AT)gmail.com
>
>
>
>
>
>
>
>
>
>
>

Re: Mesos Style Guideline Adjustments

Posted by Bernd Mathiske <be...@mesosphere.io>.
IMHO jaggedness is a secondary concern.
I’d rather have subsentences on the same line
than try to meet some page boundary on the right consistently.
I see no value in that.

Of course, a maximum line width of 80 columns is necessary
due to anachronistic limitations of our tools.
Here is the above paragraph without jaggedness, with 80 columns
(which the email system will probably mess up before you get to see it):

IMHO jaggedness is a secondary concern. I’d rather have subsentences on the
same line  than try to meet some page boundary on the right consistently. I
see no value in that.

Here it is with 70 columns:

IMHO jaggedness is a secondary concern. I’d rather have subsentences
on the same line than try to meet some page boundary on the right
consistently. I see no value in that.

Maybe for some of us, avoiding jaggedness increases readability.
For me personally, it decreases it.

Bernd

> On Nov 6, 2015, at 8:36 PM, Kapil Arya <ka...@mesosphere.io> wrote:
> 
> As far as setting a soft limit goes, I think there are several different
> style guides and it basically boils down to personal preferences as well.
> The Linux `fmt` command wraps by 75 columns by default; others use 72 or as
> extreme as 60 columns.
> 
> Having said that, it's nice to have tools that enforce some
> check. Jaggedness isn't pretty and one can always use their judgement to
> reformat the comments to make it look nicer and more readable.
> 
> My 2 cents.
> 
> On Fri, Nov 6, 2015 at 2:26 PM, Joris Van Remoortere <jo...@mesosphere.io>
> wrote:
> 
>> @ben Is "jaggedness" the only *formatting* influence on the readability of
>> the comments? If so, would it be possible to make a simple github.io page
>> where we can paste comments, and they get formatted for minimal jaggedness
>> based on some simple math? This would help provide consistency between
>> contributions, and likely better results than humans trying to optimize the
>> jaggedness equation.
>> 
>> This way we can focus on the content of the comments when reviewing, rather
>> than the format. Later one we might even be able to incorporate this in
>> more intelligent editor friendly formatting tools.
>> 
>> If there is more than some simple math involved, this may not be a viable
>> solution.
>> 
>> Joris
>> 
>> —
>> *Joris Van Remoortere*
>> Mesosphere
>> 
>> On Fri, Nov 6, 2015 at 11:10 AM, Benjamin Mahler <
>> benjamin.mahler@gmail.com>
>> wrote:
>> 
>>> I raise this because there is a ton of value in keeping our codebase as
>>> readable as possible. Having had to navigate and maintain the code base
>> for
>>> the past few years, this _is_ a "real issue" that deserves the
>> contributors
>>> spending their mental resources on. I realize this seems very subtle, but
>>> it is of critical importance for the maintainability of the project that
>>> folks are paying attention to how their code and comments will be read by
>>> others.
>>> 
>>> I think about this as follows: we'll always have "soft" rules that cannot
>>> be enforced by these style checkers but require mentorship to communicate
>>> as we grow the community. These tools cannot tell you how to structure
>> your
>>> code in a simple form. They also can't tell you how to write a comment
>> in a
>>> way that is clear to others.
>>> 
>>> To Alex's example, two comments:
>>> 
>>> (1) The second comment is poorly written, did no one even notice this??
>>> That's a "real issue" :(
>>> (2) It's important not to isolate the comments from the code. The
>> comments
>>> live with the code and a major reason why long line length paragraphs are
>>> irregular is because the majority of code lines do not approach 80
>>> characters.
>>> (3) We tend to separate "multiple logical parts" of a comment with an
>> empty
>>> // line, which helps readability.
>>> (4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
>>> "jaggedness" (or to increase "regularity") and (b) long line lengths (80)
>>> for large paragraphs are harder to read.
>>> 
>>> I just don't want the formatter to become a crutch that causes folks to
>>> think less about the implications of how they write their comments. Do
>> the
>>> soft rules in (a) and (b) seem reasonable? We already need to be diligent
>>> in reviews to ensure that comments are well-written.
>>> 
>>> On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
>>> benjamin.bannier@mesosphere.io> wrote:
>>> 
>>>> Hi,
>>>> 
>>>> just to echo Alexander’s point, for newbies like me being able to
>>> delegate
>>>> formatting decisions to tools as much as possible frees up a lot of
>>> mental
>>>> resources for tackling the real issues.
>>>> 
>>>> 
>>>> Cheers,
>>>> 
>>>> Benjamin
>>>> 
>>>> ps. Also looking forward to an updated and expanded clang-format
>> config.
>>>> 
>>>> 
>>>>> On Nov 6, 2015, at 1:44 PM, Alexander Rojas <alexander@mesosphere.io
>>> 
>>>> wrote:
>>>>> 
>>>>> I think one of the main reasons we move to having 80 as the limit for
>>>> both code and comments is the ability it gives us to use tools (e.g.
>>>> clang-format) to enforce formatting rules, so personally I rather have
>> us
>>>> putting effort towards that goal. On that note, the developer branch of
>>>> clang-format allows a much closer formatting options to the ones we
>> use.
>>> On
>>>> OS X it can be installed using `brew install --HEAD clang-format`.
>>>>> 
>>>>> Right now I’m working on setting the config file to be as close as
>>>> possible to our style.
>>>>> 
>>>>>> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com>
>> wrote:
>>>>>> 
>>>>>> I think jaggedness in the example you provide comes mainly from the
>>> fact
>>>>>> that the second comment has multiple logical blocks. I have
>> formatted
>>>> both
>>>>>> comments at 70 and at 80, here is the outcome:
>>>> http://pastebin.com/nRQB0nCD
>>>>>> 
>>>>>> While the first comment indeed looks better when wrapped at 70, I
>>> can't
>>>> say
>>>>>> the same about the second one.
>>>>>> 
>>>>>> I would say, that the longer a line could be, the less jagged the
>>>> comment
>>>>>> block is. The ratio (`averageWordLength` / `maxLineLength`)
>> approaches
>>>> 0 as
>>>>>> `maxLineLenght` approaches infinity, which means wrapping a long
>> word
>>>> right
>>>>>> before the line end should be perceived less jagged : ).
>>>>>> 
>>>>>> Also, the longer an individual line can be, the less total lines are
>>>> needed
>>>>>> for a comment block, which reduces jaggedness and makes code a
>> little
>>>> bit
>>>>>> more readable.
>>>>>> 
>>>>>> But my strongest argument is that having a separate soft rule for
>>>> comments
>>>>>> is hard to enforce. I think what we can do is to encourage
>>> contributors
>>>> /
>>>>>> committers to wrap comments in the most logical way—like the first
>>>> comment
>>>>>> in the example you provide—even if the line length is not fully
>>>> utilized.
>>>>>> Having said that, I would rather keep a single number: hard limit at
>>> 80
>>>> for
>>>>>> simplicity.
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
>>>> benjamin.mahler@gmail.com>
>>>>>> wrote:
>>>>>> 
>>>>>>> This has come up in a couple of reviews, seems like we should add
>>> some
>>>> soft
>>>>>>> guidelines around how to format comments for readability.
>>>>>>> 
>>>>>>> In particular, the reason that we wrapped at 70 in the past was for
>>>>>>> readability, so it would be great to continue doing so as a soft
>>>> stylistic
>>>>>>> rule. The other thing we've been doing for readability is reducing
>>>>>>> "jaggedness" (variability in line lengths).
>>>>>>> 
>>>>>>> It would be great to establish these as soft rules and encourage
>> new
>>>>>>> contributors / committers to follow them. Compare these two
>> comments
>>> in
>>>>>>> Master::updateTask. The first one wraps at 70 and reduces
>> jagedness,
>>>> the
>>>>>>> second wraps at 80 and is more jagged:
>>>>>>> 
>>>>>>> 
>>>> 
>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
>>>>>>> 
>>>> 
>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>>>>>>> 
>>>>>>> I can provide more examples to help clarify. If no one objects,
>> I'll
>>>> follow
>>>>>>> up with an update to the style guide. Thoughts appreciated!
>>>>>>> 
>>>>>>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <
>> bernd@mesosphere.io
>>>> 
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> +1
>>>>>>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com>
>> wrote:
>>>>>>>>> 
>>>>>>>>> +1
>>>>>>>>> 
>>>>>>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
>>>>>>>>> 
>>>>>>>>>> +1
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> Thanks, Michael!
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>>> —
>>>>>>>>>> Sent from my iPhone, which is not as good as you'd hope to fix
>>>> trypos
>>>>>>> n
>>>>>>>>>> abbrvtn.
>>>>>>>>>> 
>>>>>>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mcypark@gmail.com
>>> 
>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> I've removed the 70 column restriction on comments from the
>> style
>>>>>>>> guide:
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>> 
>>> 
>> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
>>>>>>>>>>> Also, based on the comments, it seems like we should allow 80
>>>> column
>>>>>>>>>>> comments but omit the sweeping change.
>>>>>>>>>>> Thanks,
>>>>>>>>>>> MPark.
>>>>>>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
>>>> marco@mesosphere.io
>>>>>>>> 
>>>>>>>>>> wrote:
>>>>>>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
>>>>>>> bernd@mesosphere.io>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> Like BenM,
>>>>>>>>>>>>> 
>>>>>>>>>>>>> +1 on allowing 80 column comments
>>>>>>>>>>>>> 
>>>>>>>>>>>> +1
>>>>>>>>>>>> (it really IS annoying having to keep an eye on the bottom
>>> column
>>>>>>>>>> counter
>>>>>>>>>>>> when typing comments :)
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>>> -1 on sweeping changes; incremental changes when touching old
>>>>>>>> comments
>>>>>>>>>>>>> will do IMHO
>>>>>>>>>>>>> 
>>>>>>>>>>>>> +1 on the -1? :)
>>>>>>>>>>>> Incremental changes are good and I doubt anyone will be
>>> "confused"
>>>>>>> by
>>>>>>>>>> them.
>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>>>> Bernd
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <
>> mcypark@gmail.com
>>>> 
>>>>>>>>>> wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Ben, thanks for your input!
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> Another update on this topic: the patches around break
>> before
>>>>>>> braces
>>>>>>>>>>>> for
>>>>>>>>>>>>>> *enum* style and overloaded operators have been committed.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
>>>>>>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> We already don't necessarily wrap at 70 characters (often
>> we
>>>> wrap
>>>>>>>>>>>>> before 70
>>>>>>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> So with the change to 80, this still makes all existing
>>>> comments
>>>>>>>>>>>> valid.
>>>>>>>>>>>>> We
>>>>>>>>>>>>>>> can still encourage folks to write paragraphs in a way that
>>> is
>>>>>>>>>> easy to
>>>>>>>>>>>>>>> digest for the reader. That is, I think we should still be
>>>> trying
>>>>>>>>>> not
>>>>>>>>>>>> to
>>>>>>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
>>>>>>> stylistic
>>>>>>>>>>>>>>> violation given we don't have an algorithm for this.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to
>>>> sweeping
>>>>>>>>>>>> across
>>>>>>>>>>>>> all
>>>>>>>>>>>>>>> the comments or doing wrapping based only on line length
>>> rather
>>>>>>>>>> than
>>>>>>>>>>>>>>> jaggedness going forward.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
>>>>>>>>>>>>> joris@mesosphere.io>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80
>> if
>>>> we
>>>>>>>>>> agree
>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>> keep the code base consistent.
>>>>>>>>>>>>>>>> Naturally that is also my vote ;-)
>>>>>>>>>>>>>>>> Joris
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <
>>> mcypark@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> An update on this topic since we covered it at the
>>> community
>>>>>>>>>>>> developer
>>>>>>>>>>>>>>>> sync.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
>>>> their
>>>>>>>>>>>> style
>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>> equivalent to ours. The only change this entails for our
>>>>>>>>>> codebase
>>>>>>>>>>>> is
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as
>>> we're
>>>>>>>>>>>>>>> currently
>>>>>>>>>>>>>>>>> inconsistent. I've taken on the work involved in this
>>> change:
>>>>>>>>>>>>>>>>> - stout: https://reviews.apache.org/r/37258
>>>>>>>>>>>>>>>>> - libprocess: https://reviews.apache.org/r/37259
>>>>>>>>>>>>>>>>> - mesos: https://reviews.apache.org/r/37260
>>>>>>>>>>>>>>>>> 2. We will drop the rule for adding spaces around
>>> overloaded
>>>>>>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to
>>> update
>>>>>>>>>> all of
>>>>>>>>>>>>>>>> them
>>>>>>>>>>>>>>>>> consistently. Artem has kindly taken action on this
>>> already:
>>>>>>>>>>>>>>>>> - stout: https://reviews.apache.org/r/37018/
>>>>>>>>>>>>>>>>> - libprocess: https://reviews.apache.org/r/37017/
>>>>>>>>>>>>>>>>> - mesos: https://reviews.apache.org/r/37013/
>>>>>>>>>>>>>>>>> 3. We will drop the rule for wrapping comments at 70
>>>>>>>>>> characters.
>>>>>>>>>>>>>>> We
>>>>>>>>>>>>>>>>> have a few options to proceed here:
>>>>>>>>>>>>>>>>> - Keep all the existing comments in tact, and simply
>> allow
>>>>>>>>>> new
>>>>>>>>>>>>>>>>> comments to wrap at 80, this is less work.
>>>>>>>>>>>>>>>>> - Update all instances of the comments wrapping at 70 to
>>> be
>>>>>>>>>>>>>>> wrapped
>>>>>>>>>>>>>>>>> at 80, so that we can be consistent.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at
>> 80,
>>>>>>> but I
>>>>>>>>>>>> have
>>>>>>>>>>>>>>>>> heard arguments to update the existing comments, so that
>> we
>>>> can
>>>>>>>>>> be
>>>>>>>>>>>>>>>>> consistent across the codebase. If you have a
>>>>>>> suggestion/opinion
>>>>>>>>>> on
>>>>>>>>>>>>> how
>>>>>>>>>>>>>>>> we
>>>>>>>>>>>>>>>>> should proceed with (3), please share!
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> MPark.
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
>>>>>>>>>>>>>>> alexander@mesosphere.io>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines
>> a
>>>>>>> little
>>>>>>>>>>>> bit
>>>>>>>>>>>>>>>> than
>>>>>>>>>>>>>>>>>> waiting for months
>>>>>>>>>>>>>>>>>> to get our changes into the clang-format source without
>>> any
>>>>>>>>>>>> security
>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>> it will actually land.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
>>>>>>> alex@mesosphere.com>
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> I think automation is very important. If we should
>>> slightly
>>>>>>>>>> change
>>>>>>>>>>>>>>> our
>>>>>>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I
>> vote
>>>>>>> with
>>>>>>>>>>>> both
>>>>>>>>>>>>>>>> hands
>>>>>>>>>>>>>>>>>>> for that.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
>>>>>>>>>> mcypark@gmail.com
>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just
>> solve
>>>> the
>>>>>>>>>> issue
>>>>>>>>>>>>>>>> that I
>>>>>>>>>>>>>>>>>>>> forgot to answer your question.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely
>>> used
>>>>>>>>>> styles,
>>>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>>> I'm
>>>>>>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside
>>> from
>>>>>>>>>> that,
>>>>>>>>>>>>>>>> another
>>>>>>>>>>>>>>>>>>>> concern was that it could take a while for our style
>>>>>>>>>> proposals to
>>>>>>>>>>>>>>> make
>>>>>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>>>>>> upstream and for it to be useful.
>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
>>>>>>>>>> mcypark@gmail.com>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
>> Mozilla,
>>>>>>>>>> WebKit.).
>>>>>>>>>>>>>>> How
>>>>>>>>>>>>>>>>>>>>>> hard is it?
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was
>>>> added
>>>>>>> as
>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>> newest
>>>>>>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on
>>>> enum,
>>>>>>>>>>>>>>> function,
>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we
>>> can
>>>>>>>>>>>> actually
>>>>>>>>>>>>>>>> use
>>>>>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
>>>>>>> codebase,
>>>>>>>>>> we
>>>>>>>>>>>>>>> wrap
>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It
>> would
>>>> be
>>>>>>>>>> about
>>>>>>>>>>>>> 35
>>>>>>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in
>>> our
>>>>>>>>>>>> codebase.
>>>>>>>>>>>>>>>> What
>>>>>>>>>>>>>>>>>>>> do
>>>>>>>>>>>>>>>>>>>>> you think?
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
>>>>>>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
>> Mozilla,
>>>>>>>>>>>> WebKit.).
>>>>>>>>>>>>>>> How
>>>>>>>>>>>>>>>>>>>> hard
>>>>>>>>>>>>>>>>>>>>>> is it?
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
>>>>>>>>>>>> mcypark@gmail.com
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines
>>>> that I
>>>>>>>>>>>> would
>>>>>>>>>>>>>>>> like
>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>> propose to drop. They are:
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
>>>>>>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the
>>> brace
>>>>>>>>>> after
>>>>>>>>>>>>>>>>>>>>>>> *namespace*,
>>>>>>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule
>> at
>>>> all.
>>>>>>>>>> But
>>>>>>>>>>>> it
>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
>>>>>>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with
>> spaces.
>>>>>>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would
>>>> like
>>>>>>> to
>>>>>>>>>>>>>>> reduce
>>>>>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces.
>>> This
>>>>>>> is a
>>>>>>>>>>>> dual
>>>>>>>>>>>>>>>>>>>>>> effort, as
>>>>>>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some
>>> of
>>>>>>> our
>>>>>>>>>>>> style
>>>>>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
>>>>>>> function
>>>>>>>>>>>>>>>> arguments
>>>>>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request
>>>> which
>>>>>>>>>> is
>>>>>>>>>>>>>>> being
>>>>>>>>>>>>>>>>>>>>>> tracked
>>>>>>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
>>>>>>>>>> instances
>>>>>>>>>>>> of
>>>>>>>>>>>>>>>> (1)
>>>>>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the
>>>> constraint
>>>>>>>>>> from
>>>>>>>>>>>> 70
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>> 80
>>>>>>>>>>>>>>>>>>>>>>> without touching the existing comments.
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping
>>> any
>>>>>>> of
>>>>>>>>>>>> the 3
>>>>>>>>>>>>>>>>>> rules
>>>>>>>>>>>>>>>>>>>>>>> above?
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>>>>>> MPark.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> --
>>>>>>>>> Deshi Xiao
>>>>>>>>> Twitter: xds2000
>>>>>>>>> E-mail: xiaods(AT)gmail.com
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>> 
>>>> 
>>>> 
>>> 
>> 


Re: Mesos Style Guideline Adjustments

Posted by Kapil Arya <ka...@mesosphere.io>.
As far as setting a soft limit goes, I think there are several different
style guides and it basically boils down to personal preferences as well.
The Linux `fmt` command wraps by 75 columns by default; others use 72 or as
extreme as 60 columns.

Having said that, it's nice to have tools that enforce some
check. Jaggedness isn't pretty and one can always use their judgement to
reformat the comments to make it look nicer and more readable.

My 2 cents.

On Fri, Nov 6, 2015 at 2:26 PM, Joris Van Remoortere <jo...@mesosphere.io>
wrote:

> @ben Is "jaggedness" the only *formatting* influence on the readability of
> the comments? If so, would it be possible to make a simple github.io page
> where we can paste comments, and they get formatted for minimal jaggedness
> based on some simple math? This would help provide consistency between
> contributions, and likely better results than humans trying to optimize the
> jaggedness equation.
>
> This way we can focus on the content of the comments when reviewing, rather
> than the format. Later one we might even be able to incorporate this in
> more intelligent editor friendly formatting tools.
>
> If there is more than some simple math involved, this may not be a viable
> solution.
>
> Joris
>
> —
> *Joris Van Remoortere*
> Mesosphere
>
> On Fri, Nov 6, 2015 at 11:10 AM, Benjamin Mahler <
> benjamin.mahler@gmail.com>
> wrote:
>
> > I raise this because there is a ton of value in keeping our codebase as
> > readable as possible. Having had to navigate and maintain the code base
> for
> > the past few years, this _is_ a "real issue" that deserves the
> contributors
> > spending their mental resources on. I realize this seems very subtle, but
> > it is of critical importance for the maintainability of the project that
> > folks are paying attention to how their code and comments will be read by
> > others.
> >
> > I think about this as follows: we'll always have "soft" rules that cannot
> > be enforced by these style checkers but require mentorship to communicate
> > as we grow the community. These tools cannot tell you how to structure
> your
> > code in a simple form. They also can't tell you how to write a comment
> in a
> > way that is clear to others.
> >
> > To Alex's example, two comments:
> >
> > (1) The second comment is poorly written, did no one even notice this??
> > That's a "real issue" :(
> > (2) It's important not to isolate the comments from the code. The
> comments
> > live with the code and a major reason why long line length paragraphs are
> > irregular is because the majority of code lines do not approach 80
> > characters.
> > (3) We tend to separate "multiple logical parts" of a comment with an
> empty
> > // line, which helps readability.
> > (4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
> > "jaggedness" (or to increase "regularity") and (b) long line lengths (80)
> > for large paragraphs are harder to read.
> >
> > I just don't want the formatter to become a crutch that causes folks to
> > think less about the implications of how they write their comments. Do
> the
> > soft rules in (a) and (b) seem reasonable? We already need to be diligent
> > in reviews to ensure that comments are well-written.
> >
> > On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
> > benjamin.bannier@mesosphere.io> wrote:
> >
> > > Hi,
> > >
> > > just to echo Alexander’s point, for newbies like me being able to
> > delegate
> > > formatting decisions to tools as much as possible frees up a lot of
> > mental
> > > resources for tackling the real issues.
> > >
> > >
> > > Cheers,
> > >
> > > Benjamin
> > >
> > > ps. Also looking forward to an updated and expanded clang-format
> config.
> > >
> > >
> > > > On Nov 6, 2015, at 1:44 PM, Alexander Rojas <alexander@mesosphere.io
> >
> > > wrote:
> > > >
> > > > I think one of the main reasons we move to having 80 as the limit for
> > > both code and comments is the ability it gives us to use tools (e.g.
> > > clang-format) to enforce formatting rules, so personally I rather have
> us
> > > putting effort towards that goal. On that note, the developer branch of
> > > clang-format allows a much closer formatting options to the ones we
> use.
> > On
> > > OS X it can be installed using `brew install --HEAD clang-format`.
> > > >
> > > > Right now I’m working on setting the config file to be as close as
> > > possible to our style.
> > > >
> > > >> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com>
> wrote:
> > > >>
> > > >> I think jaggedness in the example you provide comes mainly from the
> > fact
> > > >> that the second comment has multiple logical blocks. I have
> formatted
> > > both
> > > >> comments at 70 and at 80, here is the outcome:
> > > http://pastebin.com/nRQB0nCD
> > > >>
> > > >> While the first comment indeed looks better when wrapped at 70, I
> > can't
> > > say
> > > >> the same about the second one.
> > > >>
> > > >> I would say, that the longer a line could be, the less jagged the
> > > comment
> > > >> block is. The ratio (`averageWordLength` / `maxLineLength`)
> approaches
> > > 0 as
> > > >> `maxLineLenght` approaches infinity, which means wrapping a long
> word
> > > right
> > > >> before the line end should be perceived less jagged : ).
> > > >>
> > > >> Also, the longer an individual line can be, the less total lines are
> > > needed
> > > >> for a comment block, which reduces jaggedness and makes code a
> little
> > > bit
> > > >> more readable.
> > > >>
> > > >> But my strongest argument is that having a separate soft rule for
> > > comments
> > > >> is hard to enforce. I think what we can do is to encourage
> > contributors
> > > /
> > > >> committers to wrap comments in the most logical way—like the first
> > > comment
> > > >> in the example you provide—even if the line length is not fully
> > > utilized.
> > > >> Having said that, I would rather keep a single number: hard limit at
> > 80
> > > for
> > > >> simplicity.
> > > >>
> > > >>
> > > >>
> > > >> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
> > > benjamin.mahler@gmail.com>
> > > >> wrote:
> > > >>
> > > >>> This has come up in a couple of reviews, seems like we should add
> > some
> > > soft
> > > >>> guidelines around how to format comments for readability.
> > > >>>
> > > >>> In particular, the reason that we wrapped at 70 in the past was for
> > > >>> readability, so it would be great to continue doing so as a soft
> > > stylistic
> > > >>> rule. The other thing we've been doing for readability is reducing
> > > >>> "jaggedness" (variability in line lengths).
> > > >>>
> > > >>> It would be great to establish these as soft rules and encourage
> new
> > > >>> contributors / committers to follow them. Compare these two
> comments
> > in
> > > >>> Master::updateTask. The first one wraps at 70 and reduces
> jagedness,
> > > the
> > > >>> second wraps at 80 and is more jagged:
> > > >>>
> > > >>>
> > >
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
> > > >>>
> > >
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
> > > >>>
> > > >>> I can provide more examples to help clarify. If no one objects,
> I'll
> > > follow
> > > >>> up with an update to the style guide. Thoughts appreciated!
> > > >>>
> > > >>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <
> bernd@mesosphere.io
> > >
> > > >>> wrote:
> > > >>>
> > > >>>> +1
> > > >>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com>
> wrote:
> > > >>>>>
> > > >>>>> +1
> > > >>>>>
> > > >>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> > > >>>>>
> > > >>>>>> +1
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> Thanks, Michael!
> > > >>>>>>
> > > >>>>>>
> > > >>>>>>
> > > >>>>>> —
> > > >>>>>> Sent from my iPhone, which is not as good as you'd hope to fix
> > > trypos
> > > >>> n
> > > >>>>>> abbrvtn.
> > > >>>>>>
> > > >>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mcypark@gmail.com
> >
> > > >>> wrote:
> > > >>>>>>
> > > >>>>>>> I've removed the 70 column restriction on comments from the
> style
> > > >>>> guide:
> > > >>>>>>>
> > > >>>>>>
> > > >>>>
> > > >>>
> > >
> >
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> > > >>>>>>> Also, based on the comments, it seems like we should allow 80
> > > column
> > > >>>>>>> comments but omit the sweeping change.
> > > >>>>>>> Thanks,
> > > >>>>>>> MPark.
> > > >>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
> > > marco@mesosphere.io
> > > >>>>
> > > >>>>>> wrote:
> > > >>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
> > > >>> bernd@mesosphere.io>
> > > >>>>>>>> wrote:
> > > >>>>>>>>
> > > >>>>>>>>> Like BenM,
> > > >>>>>>>>>
> > > >>>>>>>>> +1 on allowing 80 column comments
> > > >>>>>>>>>
> > > >>>>>>>> +1
> > > >>>>>>>> (it really IS annoying having to keep an eye on the bottom
> > column
> > > >>>>>> counter
> > > >>>>>>>> when typing comments :)
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>> -1 on sweeping changes; incremental changes when touching old
> > > >>>> comments
> > > >>>>>>>>> will do IMHO
> > > >>>>>>>>>
> > > >>>>>>>>> +1 on the -1? :)
> > > >>>>>>>> Incremental changes are good and I doubt anyone will be
> > "confused"
> > > >>> by
> > > >>>>>> them.
> > > >>>>>>>>
> > > >>>>>>>>
> > > >>>>>>>>> Bernd
> > > >>>>>>>>>
> > > >>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <
> mcypark@gmail.com
> > >
> > > >>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>> Ben, thanks for your input!
> > > >>>>>>>>>>
> > > >>>>>>>>>> Another update on this topic: the patches around break
> before
> > > >>> braces
> > > >>>>>>>> for
> > > >>>>>>>>>> *enum* style and overloaded operators have been committed.
> > > >>>>>>>>>>
> > > >>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> > > >>>>>>>>> benjamin.mahler@gmail.com>
> > > >>>>>>>>>> wrote:
> > > >>>>>>>>>>
> > > >>>>>>>>>>> We already don't necessarily wrap at 70 characters (often
> we
> > > wrap
> > > >>>>>>>>> before 70
> > > >>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> So with the change to 80, this still makes all existing
> > > comments
> > > >>>>>>>> valid.
> > > >>>>>>>>> We
> > > >>>>>>>>>>> can still encourage folks to write paragraphs in a way that
> > is
> > > >>>>>> easy to
> > > >>>>>>>>>>> digest for the reader. That is, I think we should still be
> > > trying
> > > >>>>>> not
> > > >>>>>>>> to
> > > >>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
> > > >>> stylistic
> > > >>>>>>>>>>> violation given we don't have an algorithm for this.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to
> > > sweeping
> > > >>>>>>>> across
> > > >>>>>>>>> all
> > > >>>>>>>>>>> the comments or doing wrapping based only on line length
> > rather
> > > >>>>>> than
> > > >>>>>>>>>>> jaggedness going forward.
> > > >>>>>>>>>>>
> > > >>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> > > >>>>>>>>> joris@mesosphere.io>
> > > >>>>>>>>>>> wrote:
> > > >>>>>>>>>>>
> > > >>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80
> if
> > > we
> > > >>>>>> agree
> > > >>>>>>>>> to
> > > >>>>>>>>>>>> keep the code base consistent.
> > > >>>>>>>>>>>> Naturally that is also my vote ;-)
> > > >>>>>>>>>>>> Joris
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <
> > mcypark@gmail.com>
> > > >>>>>> wrote:
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> An update on this topic since we covered it at the
> > community
> > > >>>>>>>> developer
> > > >>>>>>>>>>>> sync.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
> > > their
> > > >>>>>>>> style
> > > >>>>>>>>>>>> is
> > > >>>>>>>>>>>>> equivalent to ours. The only change this entails for our
> > > >>>>>> codebase
> > > >>>>>>>> is
> > > >>>>>>>>>>> to
> > > >>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as
> > we're
> > > >>>>>>>>>>> currently
> > > >>>>>>>>>>>>> inconsistent. I've taken on the work involved in this
> > change:
> > > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37258
> > > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37259
> > > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37260
> > > >>>>>>>>>>>>>  2. We will drop the rule for adding spaces around
> > overloaded
> > > >>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to
> > update
> > > >>>>>> all of
> > > >>>>>>>>>>>> them
> > > >>>>>>>>>>>>> consistently. Artem has kindly taken action on this
> > already:
> > > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37018/
> > > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37017/
> > > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37013/
> > > >>>>>>>>>>>>>  3. We will drop the rule for wrapping comments at 70
> > > >>>>>> characters.
> > > >>>>>>>>>>> We
> > > >>>>>>>>>>>>> have a few options to proceed here:
> > > >>>>>>>>>>>>>  - Keep all the existing comments in tact, and simply
> allow
> > > >>>>>> new
> > > >>>>>>>>>>>>>  comments to wrap at 80, this is less work.
> > > >>>>>>>>>>>>>  - Update all instances of the comments wrapping at 70 to
> > be
> > > >>>>>>>>>>> wrapped
> > > >>>>>>>>>>>>>  at 80, so that we can be consistent.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at
> 80,
> > > >>> but I
> > > >>>>>>>> have
> > > >>>>>>>>>>>>> heard arguments to update the existing comments, so that
> we
> > > can
> > > >>>>>> be
> > > >>>>>>>>>>>>> consistent across the codebase. If you have a
> > > >>> suggestion/opinion
> > > >>>>>> on
> > > >>>>>>>>> how
> > > >>>>>>>>>>>> we
> > > >>>>>>>>>>>>> should proceed with (3), please share!
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> Thanks,
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> MPark.
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> > > >>>>>>>>>>> alexander@mesosphere.io>
> > > >>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>
> > > >>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines
> a
> > > >>> little
> > > >>>>>>>> bit
> > > >>>>>>>>>>>> than
> > > >>>>>>>>>>>>>> waiting for months
> > > >>>>>>>>>>>>>> to get our changes into the clang-format source without
> > any
> > > >>>>>>>> security
> > > >>>>>>>>>>>> that
> > > >>>>>>>>>>>>>> it will actually land.
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
> > > >>> alex@mesosphere.com>
> > > >>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>> I think automation is very important. If we should
> > slightly
> > > >>>>>> change
> > > >>>>>>>>>>> our
> > > >>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I
> vote
> > > >>> with
> > > >>>>>>>> both
> > > >>>>>>>>>>>> hands
> > > >>>>>>>>>>>>>>> for that.
> > > >>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> > > >>>>>> mcypark@gmail.com
> > > >>>>>>>>>
> > > >>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just
> solve
> > > the
> > > >>>>>> issue
> > > >>>>>>>>>>>> that I
> > > >>>>>>>>>>>>>>>> forgot to answer your question.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely
> > used
> > > >>>>>> styles,
> > > >>>>>>>>>>>> which
> > > >>>>>>>>>>>>>> I'm
> > > >>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside
> > from
> > > >>>>>> that,
> > > >>>>>>>>>>>> another
> > > >>>>>>>>>>>>>>>> concern was that it could take a while for our style
> > > >>>>>> proposals to
> > > >>>>>>>>>>> make
> > > >>>>>>>>>>>>>> it
> > > >>>>>>>>>>>>>>>> upstream and for it to be useful.
> > > >>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> > > >>>>>> mcypark@gmail.com>
> > > >>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
> Mozilla,
> > > >>>>>> WebKit.).
> > > >>>>>>>>>>> How
> > > >>>>>>>>>>>>>>>>>> hard is it?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was
> > > added
> > > >>> as
> > > >>>>>>>> the
> > > >>>>>>>>>>>>>> newest
> > > >>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on
> > > enum,
> > > >>>>>>>>>>> function,
> > > >>>>>>>>>>>>>> and
> > > >>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we
> > can
> > > >>>>>>>> actually
> > > >>>>>>>>>>>> use
> > > >>>>>>>>>>>>>>>> that
> > > >>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
> > > >>> codebase,
> > > >>>>>> we
> > > >>>>>>>>>>> wrap
> > > >>>>>>>>>>>>>> the
> > > >>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It
> would
> > > be
> > > >>>>>> about
> > > >>>>>>>>> 35
> > > >>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in
> > our
> > > >>>>>>>> codebase.
> > > >>>>>>>>>>>> What
> > > >>>>>>>>>>>>>>>> do
> > > >>>>>>>>>>>>>>>>> you think?
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> > > >>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
> > > >>>>>>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium,
> Mozilla,
> > > >>>>>>>> WebKit.).
> > > >>>>>>>>>>> How
> > > >>>>>>>>>>>>>>>> hard
> > > >>>>>>>>>>>>>>>>>> is it?
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> > > >>>>>>>> mcypark@gmail.com
> > > >>>>>>>>>>
> > > >>>>>>>>>>>>>>>> wrote:
> > > >>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines
> > > that I
> > > >>>>>>>> would
> > > >>>>>>>>>>>> like
> > > >>>>>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>>>>> propose to drop. They are:
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> > > >>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the
> > brace
> > > >>>>>> after
> > > >>>>>>>>>>>>>>>>>>> *namespace*,
> > > >>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule
> at
> > > all.
> > > >>>>>> But
> > > >>>>>>>> it
> > > >>>>>>>>>>>> is
> > > >>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
> > > >>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with
> spaces.
> > > >>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would
> > > like
> > > >>> to
> > > >>>>>>>>>>> reduce
> > > >>>>>>>>>>>>>> the
> > > >>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces.
> > This
> > > >>> is a
> > > >>>>>>>> dual
> > > >>>>>>>>>>>>>>>>>> effort, as
> > > >>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some
> > of
> > > >>> our
> > > >>>>>>>> style
> > > >>>>>>>>>>>>>> which
> > > >>>>>>>>>>>>>>>>>> is
> > > >>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
> > > >>> function
> > > >>>>>>>>>>>> arguments
> > > >>>>>>>>>>>>>>>> to
> > > >>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request
> > > which
> > > >>>>>> is
> > > >>>>>>>>>>> being
> > > >>>>>>>>>>>>>>>>>> tracked
> > > >>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> > > >>>>>> instances
> > > >>>>>>>> of
> > > >>>>>>>>>>>> (1)
> > > >>>>>>>>>>>>>>>> and
> > > >>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the
> > > constraint
> > > >>>>>> from
> > > >>>>>>>> 70
> > > >>>>>>>>>>> to
> > > >>>>>>>>>>>>>> 80
> > > >>>>>>>>>>>>>>>>>>> without touching the existing comments.
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping
> > any
> > > >>> of
> > > >>>>>>>> the 3
> > > >>>>>>>>>>>>>> rules
> > > >>>>>>>>>>>>>>>>>>> above?
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> Thanks,
> > > >>>>>>>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>>>>>> MPark.
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>>>
> > > >>>>>>>>>>>>
> > > >>>>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>>
> > > >>>>>>>>
> > > >>>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> --
> > > >>>>> Deshi Xiao
> > > >>>>> Twitter: xds2000
> > > >>>>> E-mail: xiaods(AT)gmail.com
> > > >>>>
> > > >>>>
> > > >>>
> > > >
> > >
> > >
> >
>

Re: Mesos Style Guideline Adjustments

Posted by Joris Van Remoortere <jo...@mesosphere.io>.
@ben Is "jaggedness" the only *formatting* influence on the readability of
the comments? If so, would it be possible to make a simple github.io page
where we can paste comments, and they get formatted for minimal jaggedness
based on some simple math? This would help provide consistency between
contributions, and likely better results than humans trying to optimize the
jaggedness equation.

This way we can focus on the content of the comments when reviewing, rather
than the format. Later one we might even be able to incorporate this in
more intelligent editor friendly formatting tools.

If there is more than some simple math involved, this may not be a viable
solution.

Joris

—
*Joris Van Remoortere*
Mesosphere

On Fri, Nov 6, 2015 at 11:10 AM, Benjamin Mahler <be...@gmail.com>
wrote:

> I raise this because there is a ton of value in keeping our codebase as
> readable as possible. Having had to navigate and maintain the code base for
> the past few years, this _is_ a "real issue" that deserves the contributors
> spending their mental resources on. I realize this seems very subtle, but
> it is of critical importance for the maintainability of the project that
> folks are paying attention to how their code and comments will be read by
> others.
>
> I think about this as follows: we'll always have "soft" rules that cannot
> be enforced by these style checkers but require mentorship to communicate
> as we grow the community. These tools cannot tell you how to structure your
> code in a simple form. They also can't tell you how to write a comment in a
> way that is clear to others.
>
> To Alex's example, two comments:
>
> (1) The second comment is poorly written, did no one even notice this??
> That's a "real issue" :(
> (2) It's important not to isolate the comments from the code. The comments
> live with the code and a major reason why long line length paragraphs are
> irregular is because the majority of code lines do not approach 80
> characters.
> (3) We tend to separate "multiple logical parts" of a comment with an empty
> // line, which helps readability.
> (4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
> "jaggedness" (or to increase "regularity") and (b) long line lengths (80)
> for large paragraphs are harder to read.
>
> I just don't want the formatter to become a crutch that causes folks to
> think less about the implications of how they write their comments. Do the
> soft rules in (a) and (b) seem reasonable? We already need to be diligent
> in reviews to ensure that comments are well-written.
>
> On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
> benjamin.bannier@mesosphere.io> wrote:
>
> > Hi,
> >
> > just to echo Alexander’s point, for newbies like me being able to
> delegate
> > formatting decisions to tools as much as possible frees up a lot of
> mental
> > resources for tackling the real issues.
> >
> >
> > Cheers,
> >
> > Benjamin
> >
> > ps. Also looking forward to an updated and expanded clang-format config.
> >
> >
> > > On Nov 6, 2015, at 1:44 PM, Alexander Rojas <al...@mesosphere.io>
> > wrote:
> > >
> > > I think one of the main reasons we move to having 80 as the limit for
> > both code and comments is the ability it gives us to use tools (e.g.
> > clang-format) to enforce formatting rules, so personally I rather have us
> > putting effort towards that goal. On that note, the developer branch of
> > clang-format allows a much closer formatting options to the ones we use.
> On
> > OS X it can be installed using `brew install --HEAD clang-format`.
> > >
> > > Right now I’m working on setting the config file to be as close as
> > possible to our style.
> > >
> > >> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com> wrote:
> > >>
> > >> I think jaggedness in the example you provide comes mainly from the
> fact
> > >> that the second comment has multiple logical blocks. I have formatted
> > both
> > >> comments at 70 and at 80, here is the outcome:
> > http://pastebin.com/nRQB0nCD
> > >>
> > >> While the first comment indeed looks better when wrapped at 70, I
> can't
> > say
> > >> the same about the second one.
> > >>
> > >> I would say, that the longer a line could be, the less jagged the
> > comment
> > >> block is. The ratio (`averageWordLength` / `maxLineLength`) approaches
> > 0 as
> > >> `maxLineLenght` approaches infinity, which means wrapping a long word
> > right
> > >> before the line end should be perceived less jagged : ).
> > >>
> > >> Also, the longer an individual line can be, the less total lines are
> > needed
> > >> for a comment block, which reduces jaggedness and makes code a little
> > bit
> > >> more readable.
> > >>
> > >> But my strongest argument is that having a separate soft rule for
> > comments
> > >> is hard to enforce. I think what we can do is to encourage
> contributors
> > /
> > >> committers to wrap comments in the most logical way—like the first
> > comment
> > >> in the example you provide—even if the line length is not fully
> > utilized.
> > >> Having said that, I would rather keep a single number: hard limit at
> 80
> > for
> > >> simplicity.
> > >>
> > >>
> > >>
> > >> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
> > benjamin.mahler@gmail.com>
> > >> wrote:
> > >>
> > >>> This has come up in a couple of reviews, seems like we should add
> some
> > soft
> > >>> guidelines around how to format comments for readability.
> > >>>
> > >>> In particular, the reason that we wrapped at 70 in the past was for
> > >>> readability, so it would be great to continue doing so as a soft
> > stylistic
> > >>> rule. The other thing we've been doing for readability is reducing
> > >>> "jaggedness" (variability in line lengths).
> > >>>
> > >>> It would be great to establish these as soft rules and encourage new
> > >>> contributors / committers to follow them. Compare these two comments
> in
> > >>> Master::updateTask. The first one wraps at 70 and reduces jagedness,
> > the
> > >>> second wraps at 80 and is more jagged:
> > >>>
> > >>>
> > https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
> > >>>
> > https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
> > >>>
> > >>> I can provide more examples to help clarify. If no one objects, I'll
> > follow
> > >>> up with an update to the style guide. Thoughts appreciated!
> > >>>
> > >>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <bernd@mesosphere.io
> >
> > >>> wrote:
> > >>>
> > >>>> +1
> > >>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
> > >>>>>
> > >>>>> +1
> > >>>>>
> > >>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> > >>>>>
> > >>>>>> +1
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> Thanks, Michael!
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> —
> > >>>>>> Sent from my iPhone, which is not as good as you'd hope to fix
> > trypos
> > >>> n
> > >>>>>> abbrvtn.
> > >>>>>>
> > >>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com>
> > >>> wrote:
> > >>>>>>
> > >>>>>>> I've removed the 70 column restriction on comments from the style
> > >>>> guide:
> > >>>>>>>
> > >>>>>>
> > >>>>
> > >>>
> >
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> > >>>>>>> Also, based on the comments, it seems like we should allow 80
> > column
> > >>>>>>> comments but omit the sweeping change.
> > >>>>>>> Thanks,
> > >>>>>>> MPark.
> > >>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
> > marco@mesosphere.io
> > >>>>
> > >>>>>> wrote:
> > >>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
> > >>> bernd@mesosphere.io>
> > >>>>>>>> wrote:
> > >>>>>>>>
> > >>>>>>>>> Like BenM,
> > >>>>>>>>>
> > >>>>>>>>> +1 on allowing 80 column comments
> > >>>>>>>>>
> > >>>>>>>> +1
> > >>>>>>>> (it really IS annoying having to keep an eye on the bottom
> column
> > >>>>>> counter
> > >>>>>>>> when typing comments :)
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>> -1 on sweeping changes; incremental changes when touching old
> > >>>> comments
> > >>>>>>>>> will do IMHO
> > >>>>>>>>>
> > >>>>>>>>> +1 on the -1? :)
> > >>>>>>>> Incremental changes are good and I doubt anyone will be
> "confused"
> > >>> by
> > >>>>>> them.
> > >>>>>>>>
> > >>>>>>>>
> > >>>>>>>>> Bernd
> > >>>>>>>>>
> > >>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mcypark@gmail.com
> >
> > >>>>>> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>> Ben, thanks for your input!
> > >>>>>>>>>>
> > >>>>>>>>>> Another update on this topic: the patches around break before
> > >>> braces
> > >>>>>>>> for
> > >>>>>>>>>> *enum* style and overloaded operators have been committed.
> > >>>>>>>>>>
> > >>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> > >>>>>>>>> benjamin.mahler@gmail.com>
> > >>>>>>>>>> wrote:
> > >>>>>>>>>>
> > >>>>>>>>>>> We already don't necessarily wrap at 70 characters (often we
> > wrap
> > >>>>>>>>> before 70
> > >>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
> > >>>>>>>>>>>
> > >>>>>>>>>>> So with the change to 80, this still makes all existing
> > comments
> > >>>>>>>> valid.
> > >>>>>>>>> We
> > >>>>>>>>>>> can still encourage folks to write paragraphs in a way that
> is
> > >>>>>> easy to
> > >>>>>>>>>>> digest for the reader. That is, I think we should still be
> > trying
> > >>>>>> not
> > >>>>>>>> to
> > >>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
> > >>> stylistic
> > >>>>>>>>>>> violation given we don't have an algorithm for this.
> > >>>>>>>>>>>
> > >>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to
> > sweeping
> > >>>>>>>> across
> > >>>>>>>>> all
> > >>>>>>>>>>> the comments or doing wrapping based only on line length
> rather
> > >>>>>> than
> > >>>>>>>>>>> jaggedness going forward.
> > >>>>>>>>>>>
> > >>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> > >>>>>>>>> joris@mesosphere.io>
> > >>>>>>>>>>> wrote:
> > >>>>>>>>>>>
> > >>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80 if
> > we
> > >>>>>> agree
> > >>>>>>>>> to
> > >>>>>>>>>>>> keep the code base consistent.
> > >>>>>>>>>>>> Naturally that is also my vote ;-)
> > >>>>>>>>>>>> Joris
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <
> mcypark@gmail.com>
> > >>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> An update on this topic since we covered it at the
> community
> > >>>>>>>> developer
> > >>>>>>>>>>>> sync.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
> > their
> > >>>>>>>> style
> > >>>>>>>>>>>> is
> > >>>>>>>>>>>>> equivalent to ours. The only change this entails for our
> > >>>>>> codebase
> > >>>>>>>> is
> > >>>>>>>>>>> to
> > >>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as
> we're
> > >>>>>>>>>>> currently
> > >>>>>>>>>>>>> inconsistent. I've taken on the work involved in this
> change:
> > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37258
> > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37259
> > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37260
> > >>>>>>>>>>>>>  2. We will drop the rule for adding spaces around
> overloaded
> > >>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to
> update
> > >>>>>> all of
> > >>>>>>>>>>>> them
> > >>>>>>>>>>>>> consistently. Artem has kindly taken action on this
> already:
> > >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37018/
> > >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37017/
> > >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37013/
> > >>>>>>>>>>>>>  3. We will drop the rule for wrapping comments at 70
> > >>>>>> characters.
> > >>>>>>>>>>> We
> > >>>>>>>>>>>>> have a few options to proceed here:
> > >>>>>>>>>>>>>  - Keep all the existing comments in tact, and simply allow
> > >>>>>> new
> > >>>>>>>>>>>>>  comments to wrap at 80, this is less work.
> > >>>>>>>>>>>>>  - Update all instances of the comments wrapping at 70 to
> be
> > >>>>>>>>>>> wrapped
> > >>>>>>>>>>>>>  at 80, so that we can be consistent.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at 80,
> > >>> but I
> > >>>>>>>> have
> > >>>>>>>>>>>>> heard arguments to update the existing comments, so that we
> > can
> > >>>>>> be
> > >>>>>>>>>>>>> consistent across the codebase. If you have a
> > >>> suggestion/opinion
> > >>>>>> on
> > >>>>>>>>> how
> > >>>>>>>>>>>> we
> > >>>>>>>>>>>>> should proceed with (3), please share!
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> MPark.
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> > >>>>>>>>>>> alexander@mesosphere.io>
> > >>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines a
> > >>> little
> > >>>>>>>> bit
> > >>>>>>>>>>>> than
> > >>>>>>>>>>>>>> waiting for months
> > >>>>>>>>>>>>>> to get our changes into the clang-format source without
> any
> > >>>>>>>> security
> > >>>>>>>>>>>> that
> > >>>>>>>>>>>>>> it will actually land.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
> > >>> alex@mesosphere.com>
> > >>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> I think automation is very important. If we should
> slightly
> > >>>>>> change
> > >>>>>>>>>>> our
> > >>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I vote
> > >>> with
> > >>>>>>>> both
> > >>>>>>>>>>>> hands
> > >>>>>>>>>>>>>>> for that.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> > >>>>>> mcypark@gmail.com
> > >>>>>>>>>
> > >>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just solve
> > the
> > >>>>>> issue
> > >>>>>>>>>>>> that I
> > >>>>>>>>>>>>>>>> forgot to answer your question.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely
> used
> > >>>>>> styles,
> > >>>>>>>>>>>> which
> > >>>>>>>>>>>>>> I'm
> > >>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside
> from
> > >>>>>> that,
> > >>>>>>>>>>>> another
> > >>>>>>>>>>>>>>>> concern was that it could take a while for our style
> > >>>>>> proposals to
> > >>>>>>>>>>> make
> > >>>>>>>>>>>>>> it
> > >>>>>>>>>>>>>>>> upstream and for it to be useful.
> > >>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> > >>>>>> mcypark@gmail.com>
> > >>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> > >>>>>> WebKit.).
> > >>>>>>>>>>> How
> > >>>>>>>>>>>>>>>>>> hard is it?
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was
> > added
> > >>> as
> > >>>>>>>> the
> > >>>>>>>>>>>>>> newest
> > >>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on
> > enum,
> > >>>>>>>>>>> function,
> > >>>>>>>>>>>>>> and
> > >>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we
> can
> > >>>>>>>> actually
> > >>>>>>>>>>>> use
> > >>>>>>>>>>>>>>>> that
> > >>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
> > >>> codebase,
> > >>>>>> we
> > >>>>>>>>>>> wrap
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It would
> > be
> > >>>>>> about
> > >>>>>>>>> 35
> > >>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in
> our
> > >>>>>>>> codebase.
> > >>>>>>>>>>>> What
> > >>>>>>>>>>>>>>>> do
> > >>>>>>>>>>>>>>>>> you think?
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> > >>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
> > >>>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> Is it worth adding our own style?
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> > >>>>>>>> WebKit.).
> > >>>>>>>>>>> How
> > >>>>>>>>>>>>>>>> hard
> > >>>>>>>>>>>>>>>>>> is it?
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> > >>>>>>>> mcypark@gmail.com
> > >>>>>>>>>>
> > >>>>>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines
> > that I
> > >>>>>>>> would
> > >>>>>>>>>>>> like
> > >>>>>>>>>>>>>>>> to
> > >>>>>>>>>>>>>>>>>>> propose to drop. They are:
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> > >>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the
> brace
> > >>>>>> after
> > >>>>>>>>>>>>>>>>>>> *namespace*,
> > >>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at
> > all.
> > >>>>>> But
> > >>>>>>>> it
> > >>>>>>>>>>>> is
> > >>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
> > >>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
> > >>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would
> > like
> > >>> to
> > >>>>>>>>>>> reduce
> > >>>>>>>>>>>>>> the
> > >>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces.
> This
> > >>> is a
> > >>>>>>>> dual
> > >>>>>>>>>>>>>>>>>> effort, as
> > >>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some
> of
> > >>> our
> > >>>>>>>> style
> > >>>>>>>>>>>>>> which
> > >>>>>>>>>>>>>>>>>> is
> > >>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
> > >>> function
> > >>>>>>>>>>>> arguments
> > >>>>>>>>>>>>>>>> to
> > >>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request
> > which
> > >>>>>> is
> > >>>>>>>>>>> being
> > >>>>>>>>>>>>>>>>>> tracked
> > >>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> > >>>>>> instances
> > >>>>>>>> of
> > >>>>>>>>>>>> (1)
> > >>>>>>>>>>>>>>>> and
> > >>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the
> > constraint
> > >>>>>> from
> > >>>>>>>> 70
> > >>>>>>>>>>> to
> > >>>>>>>>>>>>>> 80
> > >>>>>>>>>>>>>>>>>>> without touching the existing comments.
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping
> any
> > >>> of
> > >>>>>>>> the 3
> > >>>>>>>>>>>>>> rules
> > >>>>>>>>>>>>>>>>>>> above?
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>>>>>> MPark.
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>
> > >>>>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> --
> > >>>>> Deshi Xiao
> > >>>>> Twitter: xds2000
> > >>>>> E-mail: xiaods(AT)gmail.com
> > >>>>
> > >>>>
> > >>>
> > >
> >
> >
>

Re: Mesos Style Guideline Adjustments

Posted by Benjamin Mahler <be...@gmail.com>.
I raise this because there is a ton of value in keeping our codebase as
readable as possible. Having had to navigate and maintain the code base for
the past few years, this _is_ a "real issue" that deserves the contributors
spending their mental resources on. I realize this seems very subtle, but
it is of critical importance for the maintainability of the project that
folks are paying attention to how their code and comments will be read by
others.

I think about this as follows: we'll always have "soft" rules that cannot
be enforced by these style checkers but require mentorship to communicate
as we grow the community. These tools cannot tell you how to structure your
code in a simple form. They also can't tell you how to write a comment in a
way that is clear to others.

To Alex's example, two comments:

(1) The second comment is poorly written, did no one even notice this??
That's a "real issue" :(
(2) It's important not to isolate the comments from the code. The comments
live with the code and a major reason why long line length paragraphs are
irregular is because the majority of code lines do not approach 80
characters.
(3) We tend to separate "multiple logical parts" of a comment with an empty
// line, which helps readability.
(4) I'm not saying wrap at 70 or at 80, but (a) write and wrap to reduce
"jaggedness" (or to increase "regularity") and (b) long line lengths (80)
for large paragraphs are harder to read.

I just don't want the formatter to become a crutch that causes folks to
think less about the implications of how they write their comments. Do the
soft rules in (a) and (b) seem reasonable? We already need to be diligent
in reviews to ensure that comments are well-written.

On Fri, Nov 6, 2015 at 8:44 AM, Benjamin Bannier <
benjamin.bannier@mesosphere.io> wrote:

> Hi,
>
> just to echo Alexander’s point, for newbies like me being able to delegate
> formatting decisions to tools as much as possible frees up a lot of mental
> resources for tackling the real issues.
>
>
> Cheers,
>
> Benjamin
>
> ps. Also looking forward to an updated and expanded clang-format config.
>
>
> > On Nov 6, 2015, at 1:44 PM, Alexander Rojas <al...@mesosphere.io>
> wrote:
> >
> > I think one of the main reasons we move to having 80 as the limit for
> both code and comments is the ability it gives us to use tools (e.g.
> clang-format) to enforce formatting rules, so personally I rather have us
> putting effort towards that goal. On that note, the developer branch of
> clang-format allows a much closer formatting options to the ones we use. On
> OS X it can be installed using `brew install --HEAD clang-format`.
> >
> > Right now I’m working on setting the config file to be as close as
> possible to our style.
> >
> >> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com> wrote:
> >>
> >> I think jaggedness in the example you provide comes mainly from the fact
> >> that the second comment has multiple logical blocks. I have formatted
> both
> >> comments at 70 and at 80, here is the outcome:
> http://pastebin.com/nRQB0nCD
> >>
> >> While the first comment indeed looks better when wrapped at 70, I can't
> say
> >> the same about the second one.
> >>
> >> I would say, that the longer a line could be, the less jagged the
> comment
> >> block is. The ratio (`averageWordLength` / `maxLineLength`) approaches
> 0 as
> >> `maxLineLenght` approaches infinity, which means wrapping a long word
> right
> >> before the line end should be perceived less jagged : ).
> >>
> >> Also, the longer an individual line can be, the less total lines are
> needed
> >> for a comment block, which reduces jaggedness and makes code a little
> bit
> >> more readable.
> >>
> >> But my strongest argument is that having a separate soft rule for
> comments
> >> is hard to enforce. I think what we can do is to encourage contributors
> /
> >> committers to wrap comments in the most logical way—like the first
> comment
> >> in the example you provide—even if the line length is not fully
> utilized.
> >> Having said that, I would rather keep a single number: hard limit at 80
> for
> >> simplicity.
> >>
> >>
> >>
> >> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <
> benjamin.mahler@gmail.com>
> >> wrote:
> >>
> >>> This has come up in a couple of reviews, seems like we should add some
> soft
> >>> guidelines around how to format comments for readability.
> >>>
> >>> In particular, the reason that we wrapped at 70 in the past was for
> >>> readability, so it would be great to continue doing so as a soft
> stylistic
> >>> rule. The other thing we've been doing for readability is reducing
> >>> "jaggedness" (variability in line lengths).
> >>>
> >>> It would be great to establish these as soft rules and encourage new
> >>> contributors / committers to follow them. Compare these two comments in
> >>> Master::updateTask. The first one wraps at 70 and reduces jagedness,
> the
> >>> second wraps at 80 and is more jagged:
> >>>
> >>>
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
> >>>
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
> >>>
> >>> I can provide more examples to help clarify. If no one objects, I'll
> follow
> >>> up with an update to the style guide. Thoughts appreciated!
> >>>
> >>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <be...@mesosphere.io>
> >>> wrote:
> >>>
> >>>> +1
> >>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
> >>>>>
> >>>>> +1
> >>>>>
> >>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> >>>>>
> >>>>>> +1
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> Thanks, Michael!
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> —
> >>>>>> Sent from my iPhone, which is not as good as you'd hope to fix
> trypos
> >>> n
> >>>>>> abbrvtn.
> >>>>>>
> >>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com>
> >>> wrote:
> >>>>>>
> >>>>>>> I've removed the 70 column restriction on comments from the style
> >>>> guide:
> >>>>>>>
> >>>>>>
> >>>>
> >>>
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> >>>>>>> Also, based on the comments, it seems like we should allow 80
> column
> >>>>>>> comments but omit the sweeping change.
> >>>>>>> Thanks,
> >>>>>>> MPark.
> >>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <
> marco@mesosphere.io
> >>>>
> >>>>>> wrote:
> >>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
> >>> bernd@mesosphere.io>
> >>>>>>>> wrote:
> >>>>>>>>
> >>>>>>>>> Like BenM,
> >>>>>>>>>
> >>>>>>>>> +1 on allowing 80 column comments
> >>>>>>>>>
> >>>>>>>> +1
> >>>>>>>> (it really IS annoying having to keep an eye on the bottom column
> >>>>>> counter
> >>>>>>>> when typing comments :)
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> -1 on sweeping changes; incremental changes when touching old
> >>>> comments
> >>>>>>>>> will do IMHO
> >>>>>>>>>
> >>>>>>>>> +1 on the -1? :)
> >>>>>>>> Incremental changes are good and I doubt anyone will be "confused"
> >>> by
> >>>>>> them.
> >>>>>>>>
> >>>>>>>>
> >>>>>>>>> Bernd
> >>>>>>>>>
> >>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mc...@gmail.com>
> >>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>> Ben, thanks for your input!
> >>>>>>>>>>
> >>>>>>>>>> Another update on this topic: the patches around break before
> >>> braces
> >>>>>>>> for
> >>>>>>>>>> *enum* style and overloaded operators have been committed.
> >>>>>>>>>>
> >>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> >>>>>>>>> benjamin.mahler@gmail.com>
> >>>>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> We already don't necessarily wrap at 70 characters (often we
> wrap
> >>>>>>>>> before 70
> >>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
> >>>>>>>>>>>
> >>>>>>>>>>> So with the change to 80, this still makes all existing
> comments
> >>>>>>>> valid.
> >>>>>>>>> We
> >>>>>>>>>>> can still encourage folks to write paragraphs in a way that is
> >>>>>> easy to
> >>>>>>>>>>> digest for the reader. That is, I think we should still be
> trying
> >>>>>> not
> >>>>>>>> to
> >>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
> >>> stylistic
> >>>>>>>>>>> violation given we don't have an algorithm for this.
> >>>>>>>>>>>
> >>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to
> sweeping
> >>>>>>>> across
> >>>>>>>>> all
> >>>>>>>>>>> the comments or doing wrapping based only on line length rather
> >>>>>> than
> >>>>>>>>>>> jaggedness going forward.
> >>>>>>>>>>>
> >>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> >>>>>>>>> joris@mesosphere.io>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>
> >>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80 if
> we
> >>>>>> agree
> >>>>>>>>> to
> >>>>>>>>>>>> keep the code base consistent.
> >>>>>>>>>>>> Naturally that is also my vote ;-)
> >>>>>>>>>>>> Joris
> >>>>>>>>>>>>
> >>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <mc...@gmail.com>
> >>>>>> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> An update on this topic since we covered it at the community
> >>>>>>>> developer
> >>>>>>>>>>>> sync.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as
> their
> >>>>>>>> style
> >>>>>>>>>>>> is
> >>>>>>>>>>>>> equivalent to ours. The only change this entails for our
> >>>>>> codebase
> >>>>>>>> is
> >>>>>>>>>>> to
> >>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as we're
> >>>>>>>>>>> currently
> >>>>>>>>>>>>> inconsistent. I've taken on the work involved in this change:
> >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37258
> >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37259
> >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37260
> >>>>>>>>>>>>>  2. We will drop the rule for adding spaces around overloaded
> >>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to update
> >>>>>> all of
> >>>>>>>>>>>> them
> >>>>>>>>>>>>> consistently. Artem has kindly taken action on this already:
> >>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37018/
> >>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37017/
> >>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37013/
> >>>>>>>>>>>>>  3. We will drop the rule for wrapping comments at 70
> >>>>>> characters.
> >>>>>>>>>>> We
> >>>>>>>>>>>>> have a few options to proceed here:
> >>>>>>>>>>>>>  - Keep all the existing comments in tact, and simply allow
> >>>>>> new
> >>>>>>>>>>>>>  comments to wrap at 80, this is less work.
> >>>>>>>>>>>>>  - Update all instances of the comments wrapping at 70 to be
> >>>>>>>>>>> wrapped
> >>>>>>>>>>>>>  at 80, so that we can be consistent.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at 80,
> >>> but I
> >>>>>>>> have
> >>>>>>>>>>>>> heard arguments to update the existing comments, so that we
> can
> >>>>>> be
> >>>>>>>>>>>>> consistent across the codebase. If you have a
> >>> suggestion/opinion
> >>>>>> on
> >>>>>>>>> how
> >>>>>>>>>>>> we
> >>>>>>>>>>>>> should proceed with (3), please share!
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> MPark.
> >>>>>>>>>>>>>
> >>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> >>>>>>>>>>> alexander@mesosphere.io>
> >>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>
> >>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines a
> >>> little
> >>>>>>>> bit
> >>>>>>>>>>>> than
> >>>>>>>>>>>>>> waiting for months
> >>>>>>>>>>>>>> to get our changes into the clang-format source without any
> >>>>>>>> security
> >>>>>>>>>>>> that
> >>>>>>>>>>>>>> it will actually land.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
> >>> alex@mesosphere.com>
> >>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> I think automation is very important. If we should slightly
> >>>>>> change
> >>>>>>>>>>> our
> >>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I vote
> >>> with
> >>>>>>>> both
> >>>>>>>>>>>> hands
> >>>>>>>>>>>>>>> for that.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> >>>>>> mcypark@gmail.com
> >>>>>>>>>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just solve
> the
> >>>>>> issue
> >>>>>>>>>>>> that I
> >>>>>>>>>>>>>>>> forgot to answer your question.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely used
> >>>>>> styles,
> >>>>>>>>>>>> which
> >>>>>>>>>>>>>> I'm
> >>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside from
> >>>>>> that,
> >>>>>>>>>>>> another
> >>>>>>>>>>>>>>>> concern was that it could take a while for our style
> >>>>>> proposals to
> >>>>>>>>>>> make
> >>>>>>>>>>>>>> it
> >>>>>>>>>>>>>>>> upstream and for it to be useful.
> >>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> >>>>>> mcypark@gmail.com>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> Is it worth adding our own style?
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> >>>>>> WebKit.).
> >>>>>>>>>>> How
> >>>>>>>>>>>>>>>>>> hard is it?
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was
> added
> >>> as
> >>>>>>>> the
> >>>>>>>>>>>>>> newest
> >>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on
> enum,
> >>>>>>>>>>> function,
> >>>>>>>>>>>>>> and
> >>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we can
> >>>>>>>> actually
> >>>>>>>>>>>> use
> >>>>>>>>>>>>>>>> that
> >>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
> >>> codebase,
> >>>>>> we
> >>>>>>>>>>> wrap
> >>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It would
> be
> >>>>>> about
> >>>>>>>>> 35
> >>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in our
> >>>>>>>> codebase.
> >>>>>>>>>>>> What
> >>>>>>>>>>>>>>>> do
> >>>>>>>>>>>>>>>>> you think?
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> >>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
> >>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> Is it worth adding our own style?
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> >>>>>>>> WebKit.).
> >>>>>>>>>>> How
> >>>>>>>>>>>>>>>> hard
> >>>>>>>>>>>>>>>>>> is it?
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> >>>>>>>> mcypark@gmail.com
> >>>>>>>>>>
> >>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines
> that I
> >>>>>>>> would
> >>>>>>>>>>>> like
> >>>>>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>>> propose to drop. They are:
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> >>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the brace
> >>>>>> after
> >>>>>>>>>>>>>>>>>>> *namespace*,
> >>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at
> all.
> >>>>>> But
> >>>>>>>> it
> >>>>>>>>>>>> is
> >>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
> >>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
> >>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would
> like
> >>> to
> >>>>>>>>>>> reduce
> >>>>>>>>>>>>>> the
> >>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces. This
> >>> is a
> >>>>>>>> dual
> >>>>>>>>>>>>>>>>>> effort, as
> >>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some of
> >>> our
> >>>>>>>> style
> >>>>>>>>>>>>>> which
> >>>>>>>>>>>>>>>>>> is
> >>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
> >>> function
> >>>>>>>>>>>> arguments
> >>>>>>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request
> which
> >>>>>> is
> >>>>>>>>>>> being
> >>>>>>>>>>>>>>>>>> tracked
> >>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> >>>>>> instances
> >>>>>>>> of
> >>>>>>>>>>>> (1)
> >>>>>>>>>>>>>>>> and
> >>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the
> constraint
> >>>>>> from
> >>>>>>>> 70
> >>>>>>>>>>> to
> >>>>>>>>>>>>>> 80
> >>>>>>>>>>>>>>>>>>> without touching the existing comments.
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping any
> >>> of
> >>>>>>>> the 3
> >>>>>>>>>>>>>> rules
> >>>>>>>>>>>>>>>>>>> above?
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> Thanks,
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>> MPark.
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>>>
> >>>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> Deshi Xiao
> >>>>> Twitter: xds2000
> >>>>> E-mail: xiaods(AT)gmail.com
> >>>>
> >>>>
> >>>
> >
>
>

Re: Mesos Style Guideline Adjustments

Posted by Benjamin Bannier <be...@mesosphere.io>.
Hi,

just to echo Alexander’s point, for newbies like me being able to delegate formatting decisions to tools as much as possible frees up a lot of mental resources for tackling the real issues.


Cheers,

Benjamin

ps. Also looking forward to an updated and expanded clang-format config.

 
> On Nov 6, 2015, at 1:44 PM, Alexander Rojas <al...@mesosphere.io> wrote:
> 
> I think one of the main reasons we move to having 80 as the limit for both code and comments is the ability it gives us to use tools (e.g. clang-format) to enforce formatting rules, so personally I rather have us putting effort towards that goal. On that note, the developer branch of clang-format allows a much closer formatting options to the ones we use. On OS X it can be installed using `brew install --HEAD clang-format`.
> 
> Right now I’m working on setting the config file to be as close as possible to our style.
> 
>> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com> wrote:
>> 
>> I think jaggedness in the example you provide comes mainly from the fact
>> that the second comment has multiple logical blocks. I have formatted both
>> comments at 70 and at 80, here is the outcome: http://pastebin.com/nRQB0nCD
>> 
>> While the first comment indeed looks better when wrapped at 70, I can't say
>> the same about the second one.
>> 
>> I would say, that the longer a line could be, the less jagged the comment
>> block is. The ratio (`averageWordLength` / `maxLineLength`) approaches 0 as
>> `maxLineLenght` approaches infinity, which means wrapping a long word right
>> before the line end should be perceived less jagged : ).
>> 
>> Also, the longer an individual line can be, the less total lines are needed
>> for a comment block, which reduces jaggedness and makes code a little bit
>> more readable.
>> 
>> But my strongest argument is that having a separate soft rule for comments
>> is hard to enforce. I think what we can do is to encourage contributors /
>> committers to wrap comments in the most logical way—like the first comment
>> in the example you provide—even if the line length is not fully utilized.
>> Having said that, I would rather keep a single number: hard limit at 80 for
>> simplicity.
>> 
>> 
>> 
>> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <be...@gmail.com>
>> wrote:
>> 
>>> This has come up in a couple of reviews, seems like we should add some soft
>>> guidelines around how to format comments for readability.
>>> 
>>> In particular, the reason that we wrapped at 70 in the past was for
>>> readability, so it would be great to continue doing so as a soft stylistic
>>> rule. The other thing we've been doing for readability is reducing
>>> "jaggedness" (variability in line lengths).
>>> 
>>> It would be great to establish these as soft rules and encourage new
>>> contributors / committers to follow them. Compare these two comments in
>>> Master::updateTask. The first one wraps at 70 and reduces jagedness, the
>>> second wraps at 80 and is more jagged:
>>> 
>>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
>>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>>> 
>>> I can provide more examples to help clarify. If no one objects, I'll follow
>>> up with an update to the style guide. Thoughts appreciated!
>>> 
>>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <be...@mesosphere.io>
>>> wrote:
>>> 
>>>> +1
>>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
>>>>> 
>>>>> +1
>>>>> 
>>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
>>>>> 
>>>>>> +1
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> Thanks, Michael!
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> —
>>>>>> Sent from my iPhone, which is not as good as you'd hope to fix trypos
>>> n
>>>>>> abbrvtn.
>>>>>> 
>>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com>
>>> wrote:
>>>>>> 
>>>>>>> I've removed the 70 column restriction on comments from the style
>>>> guide:
>>>>>>> 
>>>>>> 
>>>> 
>>> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
>>>>>>> Also, based on the comments, it seems like we should allow 80 column
>>>>>>> comments but omit the sweeping change.
>>>>>>> Thanks,
>>>>>>> MPark.
>>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <marco@mesosphere.io
>>>> 
>>>>>> wrote:
>>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
>>> bernd@mesosphere.io>
>>>>>>>> wrote:
>>>>>>>> 
>>>>>>>>> Like BenM,
>>>>>>>>> 
>>>>>>>>> +1 on allowing 80 column comments
>>>>>>>>> 
>>>>>>>> +1
>>>>>>>> (it really IS annoying having to keep an eye on the bottom column
>>>>>> counter
>>>>>>>> when typing comments :)
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> -1 on sweeping changes; incremental changes when touching old
>>>> comments
>>>>>>>>> will do IMHO
>>>>>>>>> 
>>>>>>>>> +1 on the -1? :)
>>>>>>>> Incremental changes are good and I doubt anyone will be "confused"
>>> by
>>>>>> them.
>>>>>>>> 
>>>>>>>> 
>>>>>>>>> Bernd
>>>>>>>>> 
>>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mc...@gmail.com>
>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>> Ben, thanks for your input!
>>>>>>>>>> 
>>>>>>>>>> Another update on this topic: the patches around break before
>>> braces
>>>>>>>> for
>>>>>>>>>> *enum* style and overloaded operators have been committed.
>>>>>>>>>> 
>>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
>>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> We already don't necessarily wrap at 70 characters (often we wrap
>>>>>>>>> before 70
>>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
>>>>>>>>>>> 
>>>>>>>>>>> So with the change to 80, this still makes all existing comments
>>>>>>>> valid.
>>>>>>>>> We
>>>>>>>>>>> can still encourage folks to write paragraphs in a way that is
>>>>>> easy to
>>>>>>>>>>> digest for the reader. That is, I think we should still be trying
>>>>>> not
>>>>>>>> to
>>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
>>> stylistic
>>>>>>>>>>> violation given we don't have an algorithm for this.
>>>>>>>>>>> 
>>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to sweeping
>>>>>>>> across
>>>>>>>>> all
>>>>>>>>>>> the comments or doing wrapping based only on line length rather
>>>>>> than
>>>>>>>>>>> jaggedness going forward.
>>>>>>>>>>> 
>>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
>>>>>>>>> joris@mesosphere.io>
>>>>>>>>>>> wrote:
>>>>>>>>>>> 
>>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80 if we
>>>>>> agree
>>>>>>>>> to
>>>>>>>>>>>> keep the code base consistent.
>>>>>>>>>>>> Naturally that is also my vote ;-)
>>>>>>>>>>>> Joris
>>>>>>>>>>>> 
>>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <mc...@gmail.com>
>>>>>> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>> An update on this topic since we covered it at the community
>>>>>>>> developer
>>>>>>>>>>>> sync.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as their
>>>>>>>> style
>>>>>>>>>>>> is
>>>>>>>>>>>>> equivalent to ours. The only change this entails for our
>>>>>> codebase
>>>>>>>> is
>>>>>>>>>>> to
>>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as we're
>>>>>>>>>>> currently
>>>>>>>>>>>>> inconsistent. I've taken on the work involved in this change:
>>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37258
>>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37259
>>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37260
>>>>>>>>>>>>>  2. We will drop the rule for adding spaces around overloaded
>>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to update
>>>>>> all of
>>>>>>>>>>>> them
>>>>>>>>>>>>> consistently. Artem has kindly taken action on this already:
>>>>>>>>>>>>>  - stout: https://reviews.apache.org/r/37018/
>>>>>>>>>>>>>  - libprocess: https://reviews.apache.org/r/37017/
>>>>>>>>>>>>>  - mesos: https://reviews.apache.org/r/37013/
>>>>>>>>>>>>>  3. We will drop the rule for wrapping comments at 70
>>>>>> characters.
>>>>>>>>>>> We
>>>>>>>>>>>>> have a few options to proceed here:
>>>>>>>>>>>>>  - Keep all the existing comments in tact, and simply allow
>>>>>> new
>>>>>>>>>>>>>  comments to wrap at 80, this is less work.
>>>>>>>>>>>>>  - Update all instances of the comments wrapping at 70 to be
>>>>>>>>>>> wrapped
>>>>>>>>>>>>>  at 80, so that we can be consistent.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at 80,
>>> but I
>>>>>>>> have
>>>>>>>>>>>>> heard arguments to update the existing comments, so that we can
>>>>>> be
>>>>>>>>>>>>> consistent across the codebase. If you have a
>>> suggestion/opinion
>>>>>> on
>>>>>>>>> how
>>>>>>>>>>>> we
>>>>>>>>>>>>> should proceed with (3), please share!
>>>>>>>>>>>>> 
>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>> 
>>>>>>>>>>>>> MPark.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
>>>>>>>>>>> alexander@mesosphere.io>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines a
>>> little
>>>>>>>> bit
>>>>>>>>>>>> than
>>>>>>>>>>>>>> waiting for months
>>>>>>>>>>>>>> to get our changes into the clang-format source without any
>>>>>>>> security
>>>>>>>>>>>> that
>>>>>>>>>>>>>> it will actually land.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
>>> alex@mesosphere.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> I think automation is very important. If we should slightly
>>>>>> change
>>>>>>>>>>> our
>>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I vote
>>> with
>>>>>>>> both
>>>>>>>>>>>> hands
>>>>>>>>>>>>>>> for that.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
>>>>>> mcypark@gmail.com
>>>>>>>>> 
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just solve the
>>>>>> issue
>>>>>>>>>>>> that I
>>>>>>>>>>>>>>>> forgot to answer your question.
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely used
>>>>>> styles,
>>>>>>>>>>>> which
>>>>>>>>>>>>>> I'm
>>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside from
>>>>>> that,
>>>>>>>>>>>> another
>>>>>>>>>>>>>>>> concern was that it could take a while for our style
>>>>>> proposals to
>>>>>>>>>>> make
>>>>>>>>>>>>>> it
>>>>>>>>>>>>>>>> upstream and for it to be useful.
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
>>>>>> mcypark@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
>>>>>> WebKit.).
>>>>>>>>>>> How
>>>>>>>>>>>>>>>>>> hard is it?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was added
>>> as
>>>>>>>> the
>>>>>>>>>>>>>> newest
>>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on enum,
>>>>>>>>>>> function,
>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we can
>>>>>>>> actually
>>>>>>>>>>>> use
>>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
>>> codebase,
>>>>>> we
>>>>>>>>>>> wrap
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It would be
>>>>>> about
>>>>>>>>> 35
>>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in our
>>>>>>>> codebase.
>>>>>>>>>>>> What
>>>>>>>>>>>>>>>> do
>>>>>>>>>>>>>>>>> you think?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
>>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
>>>>>>>> WebKit.).
>>>>>>>>>>> How
>>>>>>>>>>>>>>>> hard
>>>>>>>>>>>>>>>>>> is it?
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
>>>>>>>> mcypark@gmail.com
>>>>>>>>>> 
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines that I
>>>>>>>> would
>>>>>>>>>>>> like
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>> propose to drop. They are:
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
>>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the brace
>>>>>> after
>>>>>>>>>>>>>>>>>>> *namespace*,
>>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at all.
>>>>>> But
>>>>>>>> it
>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
>>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
>>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would like
>>> to
>>>>>>>>>>> reduce
>>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces. This
>>> is a
>>>>>>>> dual
>>>>>>>>>>>>>>>>>> effort, as
>>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some of
>>> our
>>>>>>>> style
>>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
>>> function
>>>>>>>>>>>> arguments
>>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request which
>>>>>> is
>>>>>>>>>>> being
>>>>>>>>>>>>>>>>>> tracked
>>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
>>>>>> instances
>>>>>>>> of
>>>>>>>>>>>> (1)
>>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the constraint
>>>>>> from
>>>>>>>> 70
>>>>>>>>>>> to
>>>>>>>>>>>>>> 80
>>>>>>>>>>>>>>>>>>> without touching the existing comments.
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping any
>>> of
>>>>>>>> the 3
>>>>>>>>>>>>>> rules
>>>>>>>>>>>>>>>>>>> above?
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>>> MPark.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> 
>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>> 
>>>>>>>>> 
>>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> Deshi Xiao
>>>>> Twitter: xds2000
>>>>> E-mail: xiaods(AT)gmail.com
>>>> 
>>>> 
>>> 
> 


Re: Mesos Style Guideline Adjustments

Posted by Alexander Rojas <al...@mesosphere.io>.
I think one of the main reasons we move to having 80 as the limit for both code and comments is the ability it gives us to use tools (e.g. clang-format) to enforce formatting rules, so personally I rather have us putting effort towards that goal. On that note, the developer branch of clang-format allows a much closer formatting options to the ones we use. On OS X it can be installed using `brew install --HEAD clang-format`.

Right now I’m working on setting the config file to be as close as possible to our style.

> On 06 Nov 2015, at 10:09, Alex Rukletsov <al...@mesosphere.com> wrote:
> 
> I think jaggedness in the example you provide comes mainly from the fact
> that the second comment has multiple logical blocks. I have formatted both
> comments at 70 and at 80, here is the outcome: http://pastebin.com/nRQB0nCD
> 
> While the first comment indeed looks better when wrapped at 70, I can't say
> the same about the second one.
> 
> I would say, that the longer a line could be, the less jagged the comment
> block is. The ratio (`averageWordLength` / `maxLineLength`) approaches 0 as
> `maxLineLenght` approaches infinity, which means wrapping a long word right
> before the line end should be perceived less jagged : ).
> 
> Also, the longer an individual line can be, the less total lines are needed
> for a comment block, which reduces jaggedness and makes code a little bit
> more readable.
> 
> But my strongest argument is that having a separate soft rule for comments
> is hard to enforce. I think what we can do is to encourage contributors /
> committers to wrap comments in the most logical way—like the first comment
> in the example you provide—even if the line length is not fully utilized.
> Having said that, I would rather keep a single number: hard limit at 80 for
> simplicity.
> 
> 
> 
> On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <be...@gmail.com>
> wrote:
> 
>> This has come up in a couple of reviews, seems like we should add some soft
>> guidelines around how to format comments for readability.
>> 
>> In particular, the reason that we wrapped at 70 in the past was for
>> readability, so it would be great to continue doing so as a soft stylistic
>> rule. The other thing we've been doing for readability is reducing
>> "jaggedness" (variability in line lengths).
>> 
>> It would be great to establish these as soft rules and encourage new
>> contributors / committers to follow them. Compare these two comments in
>> Master::updateTask. The first one wraps at 70 and reduces jagedness, the
>> second wraps at 80 and is more jagged:
>> 
>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
>> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>> 
>> I can provide more examples to help clarify. If no one objects, I'll follow
>> up with an update to the style guide. Thoughts appreciated!
>> 
>> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <be...@mesosphere.io>
>> wrote:
>> 
>>> +1
>>>> On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
>>>> 
>>>> +1
>>>> 
>>>> 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
>>>> 
>>>>> +1
>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> Thanks, Michael!
>>>>> 
>>>>> 
>>>>> 
>>>>> —
>>>>> Sent from my iPhone, which is not as good as you'd hope to fix trypos
>> n
>>>>> abbrvtn.
>>>>> 
>>>>> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com>
>> wrote:
>>>>> 
>>>>>> I've removed the 70 column restriction on comments from the style
>>> guide:
>>>>>> 
>>>>> 
>>> 
>> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
>>>>>> Also, based on the comments, it seems like we should allow 80 column
>>>>>> comments but omit the sweeping change.
>>>>>> Thanks,
>>>>>> MPark.
>>>>>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <marco@mesosphere.io
>>> 
>>>>> wrote:
>>>>>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
>> bernd@mesosphere.io>
>>>>>>> wrote:
>>>>>>> 
>>>>>>>> Like BenM,
>>>>>>>> 
>>>>>>>> +1 on allowing 80 column comments
>>>>>>>> 
>>>>>>> +1
>>>>>>> (it really IS annoying having to keep an eye on the bottom column
>>>>> counter
>>>>>>> when typing comments :)
>>>>>>> 
>>>>>>> 
>>>>>>>> -1 on sweeping changes; incremental changes when touching old
>>> comments
>>>>>>>> will do IMHO
>>>>>>>> 
>>>>>>>> +1 on the -1? :)
>>>>>>> Incremental changes are good and I doubt anyone will be "confused"
>> by
>>>>> them.
>>>>>>> 
>>>>>>> 
>>>>>>>> Bernd
>>>>>>>> 
>>>>>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mc...@gmail.com>
>>>>> wrote:
>>>>>>>>> 
>>>>>>>>> Ben, thanks for your input!
>>>>>>>>> 
>>>>>>>>> Another update on this topic: the patches around break before
>> braces
>>>>>>> for
>>>>>>>>> *enum* style and overloaded operators have been committed.
>>>>>>>>> 
>>>>>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>> wrote:
>>>>>>>>> 
>>>>>>>>>> We already don't necessarily wrap at 70 characters (often we wrap
>>>>>>>> before 70
>>>>>>>>>> to reduce "jaggedness" or to make it look cleaner).
>>>>>>>>>> 
>>>>>>>>>> So with the change to 80, this still makes all existing comments
>>>>>>> valid.
>>>>>>>> We
>>>>>>>>>> can still encourage folks to write paragraphs in a way that is
>>>>> easy to
>>>>>>>>>> digest for the reader. That is, I think we should still be trying
>>>>> not
>>>>>>> to
>>>>>>>>>> write jagged paragraphs of comments, it's just not a hard
>> stylistic
>>>>>>>>>> violation given we don't have an algorithm for this.
>>>>>>>>>> 
>>>>>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to sweeping
>>>>>>> across
>>>>>>>> all
>>>>>>>>>> the comments or doing wrapping based only on line length rather
>>>>> than
>>>>>>>>>> jaggedness going forward.
>>>>>>>>>> 
>>>>>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
>>>>>>>> joris@mesosphere.io>
>>>>>>>>>> wrote:
>>>>>>>>>> 
>>>>>>>>>>> I will volunteer to update all the comments to wrap at 80 if we
>>>>> agree
>>>>>>>> to
>>>>>>>>>>> keep the code base consistent.
>>>>>>>>>>> Naturally that is also my vote ;-)
>>>>>>>>>>> Joris
>>>>>>>>>>> 
>>>>>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <mc...@gmail.com>
>>>>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>> An update on this topic since we covered it at the community
>>>>>>> developer
>>>>>>>>>>> sync.
>>>>>>>>>>>> 
>>>>>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as their
>>>>>>> style
>>>>>>>>>>> is
>>>>>>>>>>>> equivalent to ours. The only change this entails for our
>>>>> codebase
>>>>>>> is
>>>>>>>>>> to
>>>>>>>>>>>> consistently wrap the braces for *enum* definitions, as we're
>>>>>>>>>> currently
>>>>>>>>>>>> inconsistent. I've taken on the work involved in this change:
>>>>>>>>>>>>   - stout: https://reviews.apache.org/r/37258
>>>>>>>>>>>>   - libprocess: https://reviews.apache.org/r/37259
>>>>>>>>>>>>   - mesos: https://reviews.apache.org/r/37260
>>>>>>>>>>>>   2. We will drop the rule for adding spaces around overloaded
>>>>>>>>>>>> operators. We'll simply do a sweep of the codebase to update
>>>>> all of
>>>>>>>>>>> them
>>>>>>>>>>>> consistently. Artem has kindly taken action on this already:
>>>>>>>>>>>>   - stout: https://reviews.apache.org/r/37018/
>>>>>>>>>>>>   - libprocess: https://reviews.apache.org/r/37017/
>>>>>>>>>>>>   - mesos: https://reviews.apache.org/r/37013/
>>>>>>>>>>>>   3. We will drop the rule for wrapping comments at 70
>>>>> characters.
>>>>>>>>>> We
>>>>>>>>>>>> have a few options to proceed here:
>>>>>>>>>>>>   - Keep all the existing comments in tact, and simply allow
>>>>> new
>>>>>>>>>>>>   comments to wrap at 80, this is less work.
>>>>>>>>>>>>   - Update all instances of the comments wrapping at 70 to be
>>>>>>>>>> wrapped
>>>>>>>>>>>>   at 80, so that we can be consistent.
>>>>>>>>>>>> 
>>>>>>>>>>>> I proposed that we simply allow new comments to wrap at 80,
>> but I
>>>>>>> have
>>>>>>>>>>>> heard arguments to update the existing comments, so that we can
>>>>> be
>>>>>>>>>>>> consistent across the codebase. If you have a
>> suggestion/opinion
>>>>> on
>>>>>>>> how
>>>>>>>>>>> we
>>>>>>>>>>>> should proceed with (3), please share!
>>>>>>>>>>>> 
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> 
>>>>>>>>>>>> MPark.
>>>>>>>>>>>> 
>>>>>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
>>>>>>>>>> alexander@mesosphere.io>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>> 
>>>>>>>>>>>>> I also vote up for that! I rather change our guidelines a
>> little
>>>>>>> bit
>>>>>>>>>>> than
>>>>>>>>>>>>> waiting for months
>>>>>>>>>>>>> to get our changes into the clang-format source without any
>>>>>>> security
>>>>>>>>>>> that
>>>>>>>>>>>>> it will actually land.
>>>>>>>>>>>>> 
>>>>>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
>> alex@mesosphere.com>
>>>>>>>>>> wrote:
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>> I think automation is very important. If we should slightly
>>>>> change
>>>>>>>>>> our
>>>>>>>>>>>>>> style in order to set-up easily enforceable rules, I vote
>> with
>>>>>>> both
>>>>>>>>>>> hands
>>>>>>>>>>>>>> for that.
>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
>>>>> mcypark@gmail.com
>>>>>>>> 
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> Oops, sorry I was so excited that this could just solve the
>>>>> issue
>>>>>>>>>>> that I
>>>>>>>>>>>>>>> forgot to answer your question.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>> In general, the clang-format strives to adopt widely used
>>>>> styles,
>>>>>>>>>>> which
>>>>>>>>>>>>> I'm
>>>>>>>>>>>>>>> not sure if we would be considered widely used. Aside from
>>>>> that,
>>>>>>>>>>> another
>>>>>>>>>>>>>>> concern was that it could take a while for our style
>>>>> proposals to
>>>>>>>>>> make
>>>>>>>>>>>>> it
>>>>>>>>>>>>>>> upstream and for it to be useful.
>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
>>>>> mcypark@gmail.com>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
>>>>> WebKit.).
>>>>>>>>>> How
>>>>>>>>>>>>>>>>> hard is it?
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> I was just looking into this again and *Mozilla* was added
>> as
>>>>>>> the
>>>>>>>>>>>>> newest
>>>>>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on enum,
>>>>>>>>>> function,
>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>> record definitions (struct, class, union). I think we can
>>>>>>> actually
>>>>>>>>>>> use
>>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>>> one and be done with it. Having looked through the
>> codebase,
>>>>> we
>>>>>>>>>> wrap
>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>> braces for *enum* for about half of the cases. It would be
>>>>> about
>>>>>>>> 35
>>>>>>>>>>>>>>>> instances that we have to fix from what I can see in our
>>>>>>> codebase.
>>>>>>>>>>> What
>>>>>>>>>>>>>>> do
>>>>>>>>>>>>>>>> you think?
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
>>>>>>>>>>>>>>> benjamin.mahler@gmail.com>
>>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> Is it worth adding our own style?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
>>>>>>> WebKit.).
>>>>>>>>>> How
>>>>>>>>>>>>>>> hard
>>>>>>>>>>>>>>>>> is it?
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
>>>>>>> mcypark@gmail.com
>>>>>>>>> 
>>>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines that I
>>>>>>> would
>>>>>>>>>>> like
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>> propose to drop. They are:
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
>>>>>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the brace
>>>>> after
>>>>>>>>>>>>>>>>>> *namespace*,
>>>>>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at all.
>>>>> But
>>>>>>> it
>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>> consistent throughout the codebase.
>>>>>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
>>>>>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> The main motivation is that as a community we would like
>> to
>>>>>>>>>> reduce
>>>>>>>>>>>>> the
>>>>>>>>>>>>>>>>>> discrepancy between what *clang-format* produces. This
>> is a
>>>>>>> dual
>>>>>>>>>>>>>>>>> effort, as
>>>>>>>>>>>>>>>>>> we work on improving *clang-format* to support some of
>> our
>>>>>>> style
>>>>>>>>>>>>> which
>>>>>>>>>>>>>>>>> is
>>>>>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
>> function
>>>>>>>>>>> arguments
>>>>>>>>>>>>>>> to
>>>>>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request which
>>>>> is
>>>>>>>>>> being
>>>>>>>>>>>>>>>>> tracked
>>>>>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
>>>>> instances
>>>>>>> of
>>>>>>>>>>> (1)
>>>>>>>>>>>>>>> and
>>>>>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the constraint
>>>>> from
>>>>>>> 70
>>>>>>>>>> to
>>>>>>>>>>>>> 80
>>>>>>>>>>>>>>>>>> without touching the existing comments.
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping any
>> of
>>>>>>> the 3
>>>>>>>>>>>>> rules
>>>>>>>>>>>>>>>>>> above?
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> Thanks,
>>>>>>>>>>>>>>>>>> 
>>>>>>>>>>>>>>>>>> MPark.
>>>>>>>>>>>>> 
>>>>>>>>>>>>> 
>>>>>>>>>>> 
>>>>>>>>>> 
>>>>>>>> 
>>>>>>>> 
>>>>>>> 
>>>>> 
>>>> 
>>>> 
>>>> 
>>>> --
>>>> Deshi Xiao
>>>> Twitter: xds2000
>>>> E-mail: xiaods(AT)gmail.com
>>> 
>>> 
>> 


Re: Mesos Style Guideline Adjustments

Posted by Alex Rukletsov <al...@mesosphere.com>.
I think jaggedness in the example you provide comes mainly from the fact
that the second comment has multiple logical blocks. I have formatted both
comments at 70 and at 80, here is the outcome: http://pastebin.com/nRQB0nCD

While the first comment indeed looks better when wrapped at 70, I can't say
the same about the second one.

I would say, that the longer a line could be, the less jagged the comment
block is. The ratio (`averageWordLength` / `maxLineLength`) approaches 0 as
`maxLineLenght` approaches infinity, which means wrapping a long word right
before the line end should be perceived less jagged : ).

Also, the longer an individual line can be, the less total lines are needed
for a comment block, which reduces jaggedness and makes code a little bit
more readable.

But my strongest argument is that having a separate soft rule for comments
is hard to enforce. I think what we can do is to encourage contributors /
committers to wrap comments in the most logical way—like the first comment
in the example you provide—even if the line length is not fully utilized.
Having said that, I would rather keep a single number: hard limit at 80 for
simplicity.



On Thu, Nov 5, 2015 at 10:15 PM, Benjamin Mahler <be...@gmail.com>
wrote:

> This has come up in a couple of reviews, seems like we should add some soft
> guidelines around how to format comments for readability.
>
> In particular, the reason that we wrapped at 70 in the past was for
> readability, so it would be great to continue doing so as a soft stylistic
> rule. The other thing we've been doing for readability is reducing
> "jaggedness" (variability in line lengths).
>
> It would be great to establish these as soft rules and encourage new
> contributors / committers to follow them. Compare these two comments in
> Master::updateTask. The first one wraps at 70 and reduces jagedness, the
> second wraps at 80 and is more jagged:
>
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6057
> https://github.com/apache/mesos/blob/0.25.0/src/master/master.cpp#L6072
>
> I can provide more examples to help clarify. If no one objects, I'll follow
> up with an update to the style guide. Thoughts appreciated!
>
> On Thu, Sep 10, 2015 at 8:59 AM, Bernd Mathiske <be...@mesosphere.io>
> wrote:
>
> > +1
> > > On Sep 10, 2015, at 4:21 PM, tommy xiao <xi...@gmail.com> wrote:
> > >
> > > +1
> > >
> > > 2015-09-10 9:44 GMT+08:00 Marco Massenzio <ma...@mesosphere.io>:
> > >
> > >> +1
> > >>
> > >>
> > >>
> > >>
> > >> Thanks, Michael!
> > >>
> > >>
> > >>
> > >> —
> > >> Sent from my iPhone, which is not as good as you'd hope to fix trypos
> n
> > >> abbrvtn.
> > >>
> > >> On Wed, Sep 9, 2015 at 6:23 PM, Michael Park <mc...@gmail.com>
> wrote:
> > >>
> > >>> I've removed the 70 column restriction on comments from the style
> > guide:
> > >>>
> > >>
> >
> https://github.com/apache/mesos/commit/f9c2604ea97b91f8a9ec3b2863317761679b1c86
> > >>> Also, based on the comments, it seems like we should allow 80 column
> > >>> comments but omit the sweeping change.
> > >>> Thanks,
> > >>> MPark.
> > >>> On Wed, Aug 12, 2015 at 6:13 PM Marco Massenzio <marco@mesosphere.io
> >
> > >> wrote:
> > >>>> On Wed, Aug 12, 2015 at 4:09 AM, Bernd Mathiske <
> bernd@mesosphere.io>
> > >>>> wrote:
> > >>>>
> > >>>>> Like BenM,
> > >>>>>
> > >>>>> +1 on allowing 80 column comments
> > >>>>>
> > >>>> +1
> > >>>> (it really IS annoying having to keep an eye on the bottom column
> > >> counter
> > >>>> when typing comments :)
> > >>>>
> > >>>>
> > >>>>> -1 on sweeping changes; incremental changes when touching old
> > comments
> > >>>>> will do IMHO
> > >>>>>
> > >>>>> +1 on the -1? :)
> > >>>> Incremental changes are good and I doubt anyone will be "confused"
> by
> > >> them.
> > >>>>
> > >>>>
> > >>>>> Bernd
> > >>>>>
> > >>>>>> On Aug 12, 2015, at 12:51 AM, Michael Park <mc...@gmail.com>
> > >> wrote:
> > >>>>>>
> > >>>>>> Ben, thanks for your input!
> > >>>>>>
> > >>>>>> Another update on this topic: the patches around break before
> braces
> > >>>> for
> > >>>>>> *enum* style and overloaded operators have been committed.
> > >>>>>>
> > >>>>>> On Tue, Aug 11, 2015 at 6:23 PM Benjamin Mahler <
> > >>>>> benjamin.mahler@gmail.com>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> We already don't necessarily wrap at 70 characters (often we wrap
> > >>>>> before 70
> > >>>>>>> to reduce "jaggedness" or to make it look cleaner).
> > >>>>>>>
> > >>>>>>> So with the change to 80, this still makes all existing comments
> > >>>> valid.
> > >>>>> We
> > >>>>>>> can still encourage folks to write paragraphs in a way that is
> > >> easy to
> > >>>>>>> digest for the reader. That is, I think we should still be trying
> > >> not
> > >>>> to
> > >>>>>>> write jagged paragraphs of comments, it's just not a hard
> stylistic
> > >>>>>>> violation given we don't have an algorithm for this.
> > >>>>>>>
> > >>>>>>> So +1 to relaxing the hard 70 character rule, but -1 to sweeping
> > >>>> across
> > >>>>> all
> > >>>>>>> the comments or doing wrapping based only on line length rather
> > >> than
> > >>>>>>> jaggedness going forward.
> > >>>>>>>
> > >>>>>>> On Sat, Aug 8, 2015 at 3:25 PM, Joris Van Remoortere <
> > >>>>> joris@mesosphere.io>
> > >>>>>>> wrote:
> > >>>>>>>
> > >>>>>>>> I will volunteer to update all the comments to wrap at 80 if we
> > >> agree
> > >>>>> to
> > >>>>>>>> keep the code base consistent.
> > >>>>>>>> Naturally that is also my vote ;-)
> > >>>>>>>> Joris
> > >>>>>>>>
> > >>>>>>>>> On Aug 8, 2015, at 1:40 PM, Michael Park <mc...@gmail.com>
> > >> wrote:
> > >>>>>>>>>
> > >>>>>>>>> An update on this topic since we covered it at the community
> > >>>> developer
> > >>>>>>>> sync.
> > >>>>>>>>>
> > >>>>>>>>> 1. We will adopt *Mozilla*'s *BreakBeforeBraces* style as their
> > >>>> style
> > >>>>>>>> is
> > >>>>>>>>> equivalent to ours. The only change this entails for our
> > >> codebase
> > >>>> is
> > >>>>>>> to
> > >>>>>>>>> consistently wrap the braces for *enum* definitions, as we're
> > >>>>>>> currently
> > >>>>>>>>> inconsistent. I've taken on the work involved in this change:
> > >>>>>>>>>    - stout: https://reviews.apache.org/r/37258
> > >>>>>>>>>    - libprocess: https://reviews.apache.org/r/37259
> > >>>>>>>>>    - mesos: https://reviews.apache.org/r/37260
> > >>>>>>>>>    2. We will drop the rule for adding spaces around overloaded
> > >>>>>>>>> operators. We'll simply do a sweep of the codebase to update
> > >> all of
> > >>>>>>>> them
> > >>>>>>>>> consistently. Artem has kindly taken action on this already:
> > >>>>>>>>>    - stout: https://reviews.apache.org/r/37018/
> > >>>>>>>>>    - libprocess: https://reviews.apache.org/r/37017/
> > >>>>>>>>>    - mesos: https://reviews.apache.org/r/37013/
> > >>>>>>>>>    3. We will drop the rule for wrapping comments at 70
> > >> characters.
> > >>>>>>> We
> > >>>>>>>>> have a few options to proceed here:
> > >>>>>>>>>    - Keep all the existing comments in tact, and simply allow
> > >> new
> > >>>>>>>>>    comments to wrap at 80, this is less work.
> > >>>>>>>>>    - Update all instances of the comments wrapping at 70 to be
> > >>>>>>> wrapped
> > >>>>>>>>>    at 80, so that we can be consistent.
> > >>>>>>>>>
> > >>>>>>>>> I proposed that we simply allow new comments to wrap at 80,
> but I
> > >>>> have
> > >>>>>>>>> heard arguments to update the existing comments, so that we can
> > >> be
> > >>>>>>>>> consistent across the codebase. If you have a
> suggestion/opinion
> > >> on
> > >>>>> how
> > >>>>>>>> we
> > >>>>>>>>> should proceed with (3), please share!
> > >>>>>>>>>
> > >>>>>>>>> Thanks,
> > >>>>>>>>>
> > >>>>>>>>> MPark.
> > >>>>>>>>>
> > >>>>>>>>> On Mon, Aug 3, 2015 at 2:01 PM Alexander Rojas <
> > >>>>>>> alexander@mesosphere.io>
> > >>>>>>>>> wrote:
> > >>>>>>>>>
> > >>>>>>>>>> I also vote up for that! I rather change our guidelines a
> little
> > >>>> bit
> > >>>>>>>> than
> > >>>>>>>>>> waiting for months
> > >>>>>>>>>> to get our changes into the clang-format source without any
> > >>>> security
> > >>>>>>>> that
> > >>>>>>>>>> it will actually land.
> > >>>>>>>>>>
> > >>>>>>>>>>> On 31 Jul 2015, at 09:31, Alex Rukletsov <
> alex@mesosphere.com>
> > >>>>>>> wrote:
> > >>>>>>>>>>>
> > >>>>>>>>>>> I think automation is very important. If we should slightly
> > >> change
> > >>>>>>> our
> > >>>>>>>>>>> style in order to set-up easily enforceable rules, I vote
> with
> > >>>> both
> > >>>>>>>> hands
> > >>>>>>>>>>> for that.
> > >>>>>>>>>>>
> > >>>>>>>>>>>> On Fri, Jul 31, 2015 at 3:25 AM, Michael Park <
> > >> mcypark@gmail.com
> > >>>>>
> > >>>>>>>> wrote:
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> Oops, sorry I was so excited that this could just solve the
> > >> issue
> > >>>>>>>> that I
> > >>>>>>>>>>>> forgot to answer your question.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>> In general, the clang-format strives to adopt widely used
> > >> styles,
> > >>>>>>>> which
> > >>>>>>>>>> I'm
> > >>>>>>>>>>>> not sure if we would be considered widely used. Aside from
> > >> that,
> > >>>>>>>> another
> > >>>>>>>>>>>> concern was that it could take a while for our style
> > >> proposals to
> > >>>>>>> make
> > >>>>>>>>>> it
> > >>>>>>>>>>>> upstream and for it to be useful.
> > >>>>>>>>>>>>
> > >>>>>>>>>>>>> On Thu, Jul 30, 2015, 6:12 PM Michael Park <
> > >> mcypark@gmail.com>
> > >>>>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> Is it worth adding our own style?
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> > >> WebKit.).
> > >>>>>>> How
> > >>>>>>>>>>>>>> hard is it?
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> I was just looking into this again and *Mozilla* was added
> as
> > >>>> the
> > >>>>>>>>>> newest
> > >>>>>>>>>>>>> *BreakBeforeBraces* style. It breaks before braces on enum,
> > >>>>>>> function,
> > >>>>>>>>>> and
> > >>>>>>>>>>>>> record definitions (struct, class, union). I think we can
> > >>>> actually
> > >>>>>>>> use
> > >>>>>>>>>>>> that
> > >>>>>>>>>>>>> one and be done with it. Having looked through the
> codebase,
> > >> we
> > >>>>>>> wrap
> > >>>>>>>>>> the
> > >>>>>>>>>>>>> braces for *enum* for about half of the cases. It would be
> > >> about
> > >>>>> 35
> > >>>>>>>>>>>>> instances that we have to fix from what I can see in our
> > >>>> codebase.
> > >>>>>>>> What
> > >>>>>>>>>>>> do
> > >>>>>>>>>>>>> you think?
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>> On Thu, Jul 30, 2015 at 5:14 PM Benjamin Mahler <
> > >>>>>>>>>>>> benjamin.mahler@gmail.com>
> > >>>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>
> > >>>>>>>>>>>>>> Is it worth adding our own style?
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> I noticed other have (LLVM, Google, Chromium, Mozilla,
> > >>>> WebKit.).
> > >>>>>>> How
> > >>>>>>>>>>>> hard
> > >>>>>>>>>>>>>> is it?
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>> On Thu, Jul 30, 2015 at 4:23 PM, Michael Park <
> > >>>> mcypark@gmail.com
> > >>>>>>
> > >>>>>>>>>>>> wrote:
> > >>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> There are a few syntactical Mesos style guidelines that I
> > >>>> would
> > >>>>>>>> like
> > >>>>>>>>>>>> to
> > >>>>>>>>>>>>>>> propose to drop. They are:
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> 1. Open braces for namespace should not be wrapped.
> > >>>>>>>>>>>>>>> *NOTE*: The Google style guide does not wrap the brace
> > >> after
> > >>>>>>>>>>>>>>> *namespace*,
> > >>>>>>>>>>>>>>> and the Mesos style guide does not mention a rule at all.
> > >> But
> > >>>> it
> > >>>>>>>> is
> > >>>>>>>>>>>>>>> consistent throughout the codebase.
> > >>>>>>>>>>>>>>> 2. Overloaded operators should be padded with spaces.
> > >>>>>>>>>>>>>>> 3. Comments should be wrapped at 70 characters.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> The main motivation is that as a community we would like
> to
> > >>>>>>> reduce
> > >>>>>>>>>> the
> > >>>>>>>>>>>>>>> discrepancy between what *clang-format* produces. This
> is a
> > >>>> dual
> > >>>>>>>>>>>>>> effort, as
> > >>>>>>>>>>>>>>> we work on improving *clang-format* to support some of
> our
> > >>>> style
> > >>>>>>>>>> which
> > >>>>>>>>>>>>>> is
> > >>>>>>>>>>>>>>> popular in the C++ community as well. Wrapping the
> function
> > >>>>>>>> arguments
> > >>>>>>>>>>>> to
> > >>>>>>>>>>>>>>> avoid "jaggedness" for example is a feature request which
> > >> is
> > >>>>>>> being
> > >>>>>>>>>>>>>> tracked
> > >>>>>>>>>>>>>>> at: https://llvm.org/bugs/show_bug.cgi?id=23422
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> Going forward, the proposal is to update all of the
> > >> instances
> > >>>> of
> > >>>>>>>> (1)
> > >>>>>>>>>>>> and
> > >>>>>>>>>>>>>>> (2) at once. For (3), we can simply relax the constraint
> > >> from
> > >>>> 70
> > >>>>>>> to
> > >>>>>>>>>> 80
> > >>>>>>>>>>>>>>> without touching the existing comments.
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> Does anyone have any strong opinions about dropping any
> of
> > >>>> the 3
> > >>>>>>>>>> rules
> > >>>>>>>>>>>>>>> above?
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> Thanks,
> > >>>>>>>>>>>>>>>
> > >>>>>>>>>>>>>>> MPark.
> > >>>>>>>>>>
> > >>>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>
> > >
> > >
> > >
> > > --
> > > Deshi Xiao
> > > Twitter: xds2000
> > > E-mail: xiaods(AT)gmail.com
> >
> >
>