[asterisk-dev] [Code Review] 3554: repotools: Get rid of duplicate JIRA-1234 #comments

[wdoekes]

> On May 22, 2014, 4:41 p.m., rmudgett wrote:
> > Only the #close should be used to auto-close issues.  Granted, it only needs to be done once because JIRA complains that it cannot close the issue because it is already closed.  (More stupid JIRA behavior :) )
> > 
> > The #comment should *not* be used for the reported by information for several reasons.  One, it is not the format used by the existing release scripts.  Two, the JIRA issue alaready has that information.  Three, you get the comment repeated for every merge.

Fine with me. However, when reading this post, I got a different impression:
http://lists.digium.com/pipermail/asterisk-dev/2014-March/066185.html

> The biggest implication of changing the commit message format is
> making sure that patch submitters are properly attributed, such that
> the release summary scripts and other parts of the Asterisk project
> pick up who actually wrote the patch. Right now, I'm proposing
> something like the following:
> 
> ASTERISK-12345 #close
> ASTERISK-12345 #comment patch my_awesome_fix.diff submitted by jdoe (license 12345)

And I believe I'm not the only one. Considering the other other commits I've seen recently. E.g.:

> ------------------------------------------------------------------------
> r411375 | mjordan | 2014-03-28 04:55:26 +0100 (vr, 28 mrt 2014) | 21 lines
> 
> chan_sip: Add MESSAGE request to allowed methods
> 
> The allowed methods advertised by chan_sip did not previously note the MESSAGE
> request. Even in Asterisk 1.8, we do accept in-dialog MESSAGE requests; we
> should advertise that we support MESSAGE requests.
> 
> ASTERISK-23504 #close
> ASTERISK-23504 #comment Reported by: Martin Kontsek
> ASTERISK-23504 #comment Patch sip.h_patch.diff uploaded by Martin Kontsek (license 6587)
> 
> Review: https://reviewboard.asterisk.org/r/3396/

So, perhaps a followup on that March post is in order..

Matt? :)


- wdoekes


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


On May 22, 2014, 10:36 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3554/
> -----------------------------------------------------------
> 
> (Updated May 22, 2014, 10:36 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Repotools
> 
> 
> Description
> -------
> 
> When merging across branches, the commit message is duplicated using the mergeXY
> command.
> 
> Unfortunately, when using the new JIRA-style ASTERISK-1234 #comments, this comment
> is now reported as many times as the merges are done.
> 
> See for example the last 4 comments of:
> https://issues.asterisk.org/jira/browse/ASTERISK-23650
> 
> 
> This patch prepends a pound (#) to the ISSUE_KEY so that it is (hopefully) not picked
> up a second time by JIRA and we only get a single comment from the primary commit.
> 
> (Also began to run the flake8 checker on svnmerge, but that turned out to be a bad
> idea.)
> 
> 
> Diffs
> -----
> 
>   /svnmerge 877 
> 
> Diff: https://reviewboard.asterisk.org/r/3554/diff/
> 
> 
> Testing
> -------
> 
> asterisk-11.x-WRITE$ svn log -r 414214 ../asterisk-1.8.x-WRITE/
> ------------------------------------------------------------------------
> r414214 | sgriepentrog | 2014-05-21 20:58:47 +0200 (wo, 21 mei 2014) | 13 lines
> 
> pbx.c: prevent potential crash from recursive replace()
> 
> Recurisve [sic] usage of replace() resulted in corruption of the
> temporary string storage and potential crash.  By changing
> the string to be allocated separtely per instance, this is
> eliminated.
> 
> ASTERISK-23650 #comment Reported by: Roel van Meer
> ASTERISK-23650 #close
> 
> Review: https://reviewboard.asterisk.org/r/3539/
> 
> 
> ------------------------------------------------------------------------
> 
> asterisk-11.x-WRITE$ svn up -r 414214
> ...
> 
> asterisk-11.x-WRITE$ merge811 414214
> ...
> 
> asterisk-11.x-WRITE$ cat ../merge.msg 
> pbx.c: prevent potential crash from recursive replace()
> 
> Recurisve [sic] usage of replace() resulted in corruption of the
> temporary string storage and potential crash.  By changing
> the string to be allocated separtely per instance, this is
> eliminated.
> 
> #ASTERISK-23650 #comment Reported by: Roel van Meer
> #ASTERISK-23650 #close
> 
> Review: https://reviewboard.asterisk.org/r/3539/
> ........
> 
> Merged revisions 414214 from http://svn.asterisk.org/svn/asterisk/branches/1.8
> 
> 
> Thanks,
> 
> wdoekes
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140522/c954c3e9/attachment-0001.html>