mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From tommy xiao <xia...@gmail.com>
Subject Re: Mesos Style Guideline Adjustments
Date Thu, 10 Sep 2015 14:21:38 GMT
+1

2015-09-10 9:44 GMT+08:00 Marco Massenzio <marco@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

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