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