r82788 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r82787‎ | r82788 | r82789 >
Date:10:56, 25 February 2011
Author:reedy
Status:ok (Comments)
Tags:
Comment:
* (bug 27703) CR should parse 'bug#25848'

Made space optional between bug and it's number, possibly including a hash or not
Modified paths:
  • /trunk/extensions/CodeReview/backend/CodeCommentLinker.php (modified) (history)
  • /trunk/extensions/CodeReview/backend/CodeRevision.php (modified) (history)
  • /trunk/extensions/CodeReview/ui/CodeReleaseNotes.php (modified) (history)

Diff [purge]

Index: trunk/extensions/CodeReview/backend/CodeRevision.php
@@ -476,7 +476,7 @@
477477 // Update bug references table...
478478 $affectedBugs = array();
479479 $m = array();
480 - if ( preg_match_all( '/\bbug #?(\d+)\b/', $this->message, $m ) ) {
 480+ if ( preg_match_all( '/\bbug ?#?(\d+)\b/', $this->message, $m ) ) {
481481 $data = array();
482482 foreach ( $m[1] as $bug ) {
483483 $data[] = array(
Index: trunk/extensions/CodeReview/backend/CodeCommentLinker.php
@@ -25,7 +25,7 @@
2626 array( $this, 'generalLink' ), $text );
2727 $text = preg_replace_callback( '/\br(\d+)\b/',
2828 array( $this, 'messageRevLink' ), $text );
29 - $text = preg_replace_callback( '/\bbug #?(\d+)\b/i',
 29+ $text = preg_replace_callback( '/\bbug ?#?(\d+)\b/i',
3030 array( $this, 'messageBugLink' ), $text );
3131 return $text;
3232 }
Index: trunk/extensions/CodeReview/ui/CodeReleaseNotes.php
@@ -138,7 +138,7 @@
139139 // Quick relevance tests (these *should* be over-inclusive a little if anything)
140140 private function isRelevant( $summary, $whole = true ) {
141141 # Fixed a bug? Mentioned a config var?
142 - if ( preg_match( '/\b(bug #?(\d+)|\$[we]g[0-9a-z]{3,50})\b/i', $summary ) ) {
 142+ if ( preg_match( '/\b(bug ?#?(\d+)|\$[we]g[0-9a-z]{3,50})\b/i', $summary ) ) {
143143 return true;
144144 }
145145 # Sanity check: summary cannot be *too* short to be useful

Sign-offs

UserFlagDate
Hasharinspected11:01, 25 February 2011

Follow-up revisions

RevisionCommit summaryAuthorDate
r82792Per r82788, factor out bug matching regexreedy13:08, 25 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r82786bug 27703...reedy10:33, 25 February 2011

Comments

#Comment by Hashar (talk | contribs)   11:01, 25 February 2011

That was fast!

#Comment by Reedy (talk | contribs)   11:02, 25 February 2011

It was easy to fix :P

#Comment by Bryan (talk | contribs)   11:05, 25 February 2011

Needs tests

#Comment by Reedy (talk | contribs)   11:08, 25 February 2011

Probably, but given the current lack of tests beyond the very basic ones....

#Comment by Hashar (talk | contribs)   11:07, 25 February 2011

While you are on it, can you make the 3 regexp the same or at least define them in a central place ? Maybe as constants.

#Comment by Reedy (talk | contribs)   11:09, 25 February 2011

I was thinking that would be a good idea.

The third however uses a combined regex. But I could just split it out :)

#Comment by Reedy (talk | contribs)   11:11, 25 February 2011

Just trying to think where the best place/class to put the constant...

Status & tagging log