[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    Re: [asterisk-dev] [Code Review] 3554: repotools: Get rid of duplicate JIRA-1234 #comments
From:       "wdoekes" <reviewboard () asterisk ! org>
Date:       2014-05-31 9:25:47
Message-ID: 20140531092547.11560.10826 () sonic ! digium ! api
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> On May 28, 2014, 10 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.

> 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


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


On May 23, 2014, 10: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, 10: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
> 
> 


[Attachment #5 (text/html)]

<html>
 <body>
  <div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;">  <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href="https://reviewboard.asterisk.org/r/3554/">https://reviewboard.asterisk.org/r/3554/</a>
  </td>
    </tr>
   </table>
   <br />










<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <p style="margin-top: 0;">On May 28th, 2014, 10 a.m. UTC, <b>wdoekes</b> \
wrote:</p>  <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; \
padding-left: 10px;">  



<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;">  <thead>
  <tr>
   <th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;">  <a \
href="https://reviewboard.asterisk.org/r/3554/diff/1/?file=58769#file58769line990" \
style="color: black; font-weight: bold; text-decoration: underline;">/svnmerge</a>  \
<span style="font-weight: normal;">

     (Diff revision 1)

    </span>
   </th>
  </tr>
 </thead>



 
 

 <tbody>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">990</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    message = \
re.sub(r&#39;^([A-Z]+-[0-9]+\s+#)&#39;, r&#39;#\1&#39;, message,</pre></td>  </tr>

  <tr>
    <th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th>  <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>  <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">991</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">              \
flags=re.MULTILINE)</pre></td>  </tr>

 </tbody>

</table>

  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Prepending &#39;#&#39; \
does not work.

I still get mails that &quot;FishEye was unable to execute the transition \
\&quot;close\&quot;&quot;.

Perhaps prepending &#39;NOT-&#39; or &#39;-&#39; or (as last ditch effort) infixing \
an extra hyphen (ASTERISK--1234) helps.

Needs testing. (Hint!)</pre>
 </blockquote>



 <p>On May 30th, 2014, 12:04 p.m. UTC, <b>Matt Jordan</b> wrote:</p>
 <blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;">  <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">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&#39;t get any additional message; although, that could be \
because I made JIRA accept &quot;Close&quot; -&gt; &quot;Close&quot; transitions \
yesterday.</pre>  </blockquote>







</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">&gt; that \
could be because I made JIRA accept &quot;Close&quot; -&gt; &quot;Close&quot; \
transitions yesterday.

Pretty sure that that&#39;s the case.

So, if you keep that setting, and we won&#39;t use #comment and friends, I can drop \
this review.</pre> <br />




<p>- wdoekes</p>


<br />
<p>On May 23rd, 2014, 10:23 a.m. UTC, wdoekes wrote:</p>








<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;">  <tr>
  <td>

<div>Review request for Asterisk Developers and Matt Jordan.</div>
<div>By wdoekes.</div>


<p style="color: grey;"><i>Updated May 23, 2014, 10:23 a.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
Repotools
</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
 <table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">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.)</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0">  <tr>
  <td>
   <pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
                break-word;">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
</pre>
  </td>
 </tr>
</table>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>/svnmerge <span style="color: grey">(877)</span></li>

</ul>

<p><a href="https://reviewboard.asterisk.org/r/3554/diff/" style="margin-left: \
3em;">View Diff</a></p>







  </td>
 </tr>
</table>








  </div>
 </body>
</html>



-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev

[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic