On Thu, 2016-12-01 at 15:59 -0800, Ben Pfaff wrote: > 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 <[email protected]> > > > Suggested-at: https://mail.openvswitch.org/pipermail/ovs-dev/2016 > > > -Nov > > > ember/325513.html > > > Signed-off-by: Ben Pfaff <[email protected]> > > > --- > > > 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.
Sounds fair on both counts. > 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. Looks good :) Acked-by: Stephen Finucane <[email protected]> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
