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

rmudgett reviewboard at asterisk.org
Tue Jun 3 10:44:31 CDT 2014



> On May 28, 2014, 5 a.m., wdoekes wrote:
> > /svnmerge, lines 990-991
> > <https://reviewboard.asterisk.org/r/3554/diff/1/?file=58769#file58769line990>
> >
> >     Prepending '#' does not work.
> >     
> >     I still get mails that "FishEye was unable to execute the transition \"close\"".
> >     
> >     Perhaps prepending 'NOT-' or '-' or (as last ditch effort) infixing an extra hyphen (ASTERISK--1234) helps.
> >     
> >     Needs testing. (Hint!)
> 
> Matt Jordan wrote:
>     So I tested this out and got the following:
>     
>     #ASTERISK-23803 #close
>     Reported by: zvision
>     patches:
>       manager.c.diff uploaded by zvision (License 5755)
>     
>     After commit, I didn't get any additional message; although, that could be because I made JIRA accept "Close" -> "Close" transitions yesterday.
> 
> wdoekes wrote:
>     > that could be because I made JIRA accept "Close" -> "Close" transitions yesterday.
>     
>     Pretty sure that that's the case.
>     
>     So, if you keep that setting, and we won't use #comment and friends, I can drop this review.
> 
> wdoekes wrote:
>     Ok.
>     
>     So, the following are parsed (according to bamboo mails, so I'm guessing that applies to all
>     atlassian stuff):
>     
>         ASTERISK-23582
>         #ASTERISK-23582
>         ~ASTERISK-23582
>     
>     And these aren't:
>     
>         -ASTERISK-23582
>         X-ASTERISK-23582
>     
>     And even though you've allowed state transitions from closed to closed, it does clutter the
>     JIRA History tab a tiny bit; see e.g. https://issues.asterisk.org/jira/browse/ASTERISK-23582
>     (History tab).

I think the -ASTERISK-23582 and X-ASTERISK-23582 don't show up because JIRA thinks they are references to unknown issues.  It would thus seem the only valid thing to do would be to remove the '#' from the #close.  However, this might cause the release scripts to not recognize the issue is closed on those branchs.

I'd say this change isn't getting us anything now that the additional #close's are accepted and the other #comment and friends are not useful to the project.


- rmudgett


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


On May 23, 2014, 5:23 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3554/
> -----------------------------------------------------------
> 
> (Updated May 23, 2014, 5:23 a.m.)
> 
> 
> Review request for Asterisk Developers and Matt Jordan.
> 
> 
> 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/20140603/69086dcd/attachment-0001.html>


More information about the asterisk-dev mailing list