mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Bernd Mathiske <be...@mesosphere.io>
Subject Re: Mesos Style Guideline Adjustments
Date Wed, 12 Aug 2015 11:09:31 GMT
Like BenM, 

+1 on allowing 80 column comments
-1 on sweeping changes; incremental changes when touching old comments will do IMHO

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.
>>>>> 
>>>>> 
>>> 
>> 


Mime
View raw message