Question on adding commit hook to enforce leading whitespace in comments

[Avinash Sridharan <av...@mesosphere.io>]

As a newbie to Apache mesos project I have run into issues of missing
whitespace in comments during reviews. Hence, wanted to see if we can add a
commit hook to detect this style violation and enforce it by not allowing
the commit to go through.

Accordingly have created a JIRA (
https://issues.apache.org/jira/browse/MESOS-4231) and have an RB out (
https://reviews.apache.org/r/41617/) being shephered by @mcypark .

The basic idea behind this patch is to add a new category in cpplint.py
called whitespace/mesos-comments and enable it in mesos-style.py.
cpplint.py already had a category called whitespace/comments but apart from
looking for missing leading whitespace in comments, the category also
enforces a strict 2 white space policy between code and trailing comments.
For e.g

foo(); // Great !!

would be flagged if we turn on this category due a single whitespace
between foo() and //. This is something that we don't follow in our code
and will generate a lot of errors if we just enable the whitespace/comments
category. Hence the new category.

@mcypark felt we should pose the question of enabling the enforcement of
this comment style here, and get opinion from the wider community. So the
questions that we would like to ask are:

a) Should we enforce the  "single space between // and comment” rule
explicitly using a git commit hook ?
b) Whether we want single space or double space for trailing comments, or
maybe leave it open-ended ??



Thanks,
Avinash
-- 
Avinash Sridharan, Mesosphere
+1 (323) 702 5245

Re: Question on adding commit hook to enforce leading whitespace in comments

[Adam Bordelon <ad...@mesosphere.io>]

a) +1 to new cpplint rule for single space after "//"
b) I'd vote for single-space before the comment too. I haven't seen
anybody do double-spaces in Mesos. The only exception to the
single-space rule would be places where we try to align trailing
comments. Not sure if we want to continue to allow that.

On Mon, Dec 21, 2015 at 5:18 PM, Avinash Sridharan
<av...@mesosphere.io> wrote:
> As a newbie to Apache mesos project I have run into issues of missing
> whitespace in comments during reviews. Hence, wanted to see if we can add a
> commit hook to detect this style violation and enforce it by not allowing
> the commit to go through.
>
> Accordingly have created a JIRA (
> https://issues.apache.org/jira/browse/MESOS-4231) and have an RB out (
> https://reviews.apache.org/r/41617/) being shephered by @mcypark .
>
> The basic idea behind this patch is to add a new category in cpplint.py
> called whitespace/mesos-comments and enable it in mesos-style.py.
> cpplint.py already had a category called whitespace/comments but apart from
> looking for missing leading whitespace in comments, the category also
> enforces a strict 2 white space policy between code and trailing comments.
> For e.g
>
> foo(); // Great !!
>
> would be flagged if we turn on this category due a single whitespace
> between foo() and //. This is something that we don't follow in our code
> and will generate a lot of errors if we just enable the whitespace/comments
> category. Hence the new category.
>
> @mcypark felt we should pose the question of enabling the enforcement of
> this comment style here, and get opinion from the wider community. So the
> questions that we would like to ask are:
>
> a) Should we enforce the  "single space between // and comment” rule
> explicitly using a git commit hook ?
> b) Whether we want single space or double space for trailing comments, or
> maybe leave it open-ended ??
>
>
>
> Thanks,
> Avinash
> --
> Avinash Sridharan, Mesosphere
> +1 (323) 702 5245