[asterisk-dev] [Code Review]: Modify merge message so commit log message comes first.

Tilghman Lesher reviewboard at asterisk.org
Thu Sep 22 03:46:50 CDT 2011



> On Sept. 21, 2011, 8:48 p.m., rmudgett wrote:
> > /svnmerge, line 85
> > <https://reviewboard.asterisk.org/r/1451/diff/2/?file=20859#file20859line85>
> >
> >     Why this change?

Mere customization.  We don't need 8 periods on a line as a separator.  It could just as easily be 3 or 4.


> On Sept. 21, 2011, 8:48 p.m., rmudgett wrote:
> > /svnmerge, line 1476
> > <https://reviewboard.asterisk.org/r/1451/diff/2/?file=20859#file20859line1476>
> >
> >     Undo this.  A blank line is supposed to separate the summary line from the message body.

Same reasoning applies here.


> On Sept. 21, 2011, 8:48 p.m., rmudgett wrote:
> > /svnmerge, line 1443
> > <https://reviewboard.asterisk.org/r/1451/diff/2/?file=20859#file20859line1443>
> >
> >     Undo this.  A blank line is supposed to separate the summary line from the message body.

While that's true for a regular commit message, I'm not convinced of the need of a blank line for a Blocked property message.  This isn't really a summary line.


> On Sept. 21, 2011, 8:48 p.m., rmudgett wrote:
> > /svnmerge, lines 1398-1410
> > <https://reviewboard.asterisk.org/r/1451/diff/2/?file=20859#file20859line1398>
> >
> >     I think a recorded merge commit should have the recorded revisions line first since the actual change should already have been merged by another commit.
> >     
> >     if record_only
> >        record merge summary line
> >        blank line
> >        recorded revision commit message
> >     else
> >        original commit message
> >        log separator
> >        merged revisions from
> >

Changed, although I reject the need for a blank line in the record.  I'm changing it to a separator, instead.


- Tilghman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1451/#review4423
-----------------------------------------------------------


On Sept. 21, 2011, 6:38 p.m., Tilghman Lesher wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1451/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2011, 6:38 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Summary
> -------
> 
> Some discussion was made several weeks ago about modifying the svnmerge log message, such that the committer's log message comes first in any merge, followed by details on where the commit was originally made.  This improves our overall log messages.  Also, the URL is modified in the message as to show the public path, instead of the commit path, which is not available to most.
> 
> 
> Diffs
> -----
> 
>   /svnmerge 795 
> 
> Diff: https://reviewboard.asterisk.org/r/1451/diff
> 
> 
> Testing
> -------
> 
> Currently, an example message is as follows (after 2 merges):
> 
> -----start message-----
> 
> 
> More silly spacing changes
> 
> .....
> Merged revisions 337353 from http://svn.asterisk.org/svn/asterisk/branches/1.8
> 
> .....
> Merged revisions 337380 from http://svn.asterisk.org/svn/asterisk/branches/10
> -----end message-----
> 
> 
> Thanks,
> 
> Tilghman
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20110922/b1d87afe/attachment.htm>


More information about the asterisk-dev mailing list