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 <alexan...@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 <a...@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.mah...@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 <xia...@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 <mcyp...@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 <mcyp...@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.mah...@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 < >>>>>>>>> jo...@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 <mcyp...@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 < >>>>>>>>>>> alexan...@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 < >>> a...@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 < >>>>>> mcyp...@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 < >>>>>> mcyp...@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.mah...@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 < >>>>>>>> mcyp...@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 >>>> >>>> >>> >
