On Thu, Dec 01, 2016 at 01:16:14PM +0000, Stephen Finucane wrote:
> On Wed, 2016-11-30 at 12:40 -0800, Ben Pfaff wrote:
> > Suggested-by: Joe Stringer <j...@ovn.org>
> > Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016-Nov
> > ember/325513.html
> > Signed-off-by: Ben Pfaff <b...@ovn.org>
> > ---
> >  CONTRIBUTING.rst | 12 ++++++++++--
> >  1 file changed, 10 insertions(+), 2 deletions(-)
> > 
> > diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
> > index 867562e..721371e 100644
> > --- a/CONTRIBUTING.rst
> > +++ b/CONTRIBUTING.rst
> > @@ -94,7 +94,11 @@ Where:
> >    multiple distinct pieces of code.
> >  
> >  ``<summary>``:
> > -  briefly describes the change.
> > +
> > +  briefly describes the change.  Use the the imperative form,
> > +  e.g. "Force SNAT for multiple gateway routers." or "Fix daemon
> > exit
> > +  for bad datapaths or flows."  This turns the summary into a
> > +  sentence, so please end it with a period.
> 
> This is super nitty (sorry), but the full stop takes up 1/50th of the
> available summary line characters [1] without adding any value :) I'd
> argue that its use certainly should be, at best, optional. If we're
> adding anything extra, I'd recommend that summary lines should be
> constrained to 50/51 characters where possible (vim does this itself).

Perhaps the period is more of a personal preference.  I'll drop it.

I find 50 characters to be almost impossible in many cases, myself, but
we can at least recommend it as something to try for.

> >  The subject, minus the ``[PATCH <n>/<m>]`` prefix, becomes the first
> > line of
> >  the commit's change log message.
> > @@ -106,7 +110,9 @@ The body of the email should start with a more
> > thorough description of the
> >  change.  This becomes the body of the commit message, following the
> > subject.
> >  There is no need to duplicate the summary given in the subject.
> >  
> > -Please limit lines in the description to 79 characters in width.
> > +Please limit lines in the description to 75 characters in
> > width.  That
> > +allows the description to format properly even when indented (e.g.
> > by
> > +"git log" or in email quotations).
> 
> Another nit (such is the way with anything coding standards'y): isn't
> 72 characters the usual recommendation? [1] Less importantly, we don't
> need to be so needlessly polite - "Limit lines..." is fine.

The default limit for "git citool" is 75.  Linus recommends 74 in the
subsurface tree.  I think 75 is a reasonable limit since "git log"
indents by 4 spaces and so 75-character lines won't wrap on an 80-column
terminal.

I don't think being polite costs us anything here.

Here's an incremental.

--8<--------------------------cut here-------------------------->8--

diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
index 721371e..2b262ca 100644
--- a/CONTRIBUTING.rst
+++ b/CONTRIBUTING.rst
@@ -97,8 +97,8 @@ Where:
 
   briefly describes the change.  Use the the imperative form,
   e.g. "Force SNAT for multiple gateway routers." or "Fix daemon exit
-  for bad datapaths or flows."  This turns the summary into a
-  sentence, so please end it with a period.
+  for bad datapaths or flows."  Try to keep the summary short, about
+  50 characters wide.
 
 The subject, minus the ``[PATCH <n>/<m>]`` prefix, becomes the first line of
 the commit's change log message.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to