You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Rall <dl...@collab.net> on 2006/02/09 22:21:59 UTC

Support "Reviewed by" in addition to "Review by" in log messages

On Thu, 09 Feb 2006, Peter N. Lundblad wrote:

> On Wed, 8 Feb 2006, Daniel Rall wrote:
> 
> > On Wed, 08 Feb 2006, Peter N. Lundblad wrote:
> >
> > > On Tue, 7 Feb 2006, Justin Erenkrantz wrote:
> > I wouldn't mind both being supported.  Peter's nitpick is important
> > because the tools/dev/contribulyze.py script that Karl wrote depends
> > upon an exact match:
> >
> > field_re = re.compile('^(Patch|Review|Suggested|Found) by:\s+(.*)')
> >
> +1.  Here, consistency isn't worth the noise. And, well, how many scripts
> depend on this? :-)

I've attached a patch which allows for both "Reviewed by" and "Review
by", effectively making the former an alias for the latter.

This implementation has the side-effect of generating HTML with log
messages which don't exactly match what's in the repository, because
canonical names (e.g. "Review by") are used in place of provided names
(e.g. "Reviewed by").


[[[
Accept "Reviewed by" as an alias for "Review by".

* tools/dev/contribulyze.py
  (Field.__init__): Accept an optional ALIAS argument which creates an
   instance field of the same name.

  (field_aliases): A dict of alias names to field names
   (e.g. "Reviewed" to "Review").

  (graze): When instantiating and using Field objects, take field
   aliases into account.
]]]

Index: tools/dev/contribulyze.py
===================================================================
--- tools/dev/contribulyze.py	(revision 18403)
+++ tools/dev/contribulyze.py	(working copy)
@@ -385,9 +385,11 @@
 
 class Field:
   """One field in one log message."""
-  def __init__(self, name):
+  def __init__(self, name, alias = None):
     # The name of this field (e.g., "Patch", "Review", etc).
     self.name = name
+    # An alias for the name of this field (e.g., "Reviewed").
+    self.alias = alias
     # A list of contributor objects, in the order in which they were
     # encountered in the field.
     self.contributors = [ ]
@@ -477,7 +479,8 @@
 log_separator = '-' * 72 + '\n'
 log_header_re = re.compile\
                 ('^(r[0-9]+) \| ([^|]+) \| ([^|]+) \| ([0-9]+)[^0-9]')
-field_re = re.compile('^(Patch|Review|Suggested|Found) by:\s+(.*)')
+field_re = re.compile('^(Patch|Review(ed)?|Suggested|Found) by:\s*(.*)')
+field_aliases = { 'Reviewed' : 'Review' }
 parenthetical_aside_re = re.compile('^\(.*\)\s*$')
 
 def graze(input):
@@ -520,10 +523,14 @@
               # We're on the first line of a field.  Parse the field.
               while m:
                 if not field:
-                  field = Field(m.group(1))
+                  ident = m.group(1)
+                  if field_aliases.has_key(ident):
+                    field = Field(field_aliases[ident], ident)
+                  else:
+                    field = Field(ident)
                 # Each line begins either with "WORD by:", or with whitespace.
                 in_field_re = re.compile('^('
-                                         + field.name
+                                         + (field.alias or field.name)
                                          + ' by:\s+|\s+)(\S.*)+')
                 m = in_field_re.match(line)
                 user, real, email = Contributor.parse(m.group(2))

Re: Support "Reviewed by" in addition to "Review by" in log messages

Posted by Sander Striker <st...@apache.org>.
Peter Samuelson wrote:
> [Garrett Rooney]
> 
>>If the script can be made to support the commonly used phrasing for
>>these things then go for it, it's so much better than being overly
>>anal about the exact wording people use.
> 
> 
> OTOH, don't go overboard.  If you write some sort of natural language
> parser that accepts almost anything, two years from now people will
> look at the varied log messages and not realise that the format is
> machine-parsed at all.  Then they'll outsmart the parser without even
> noticing it.  The status quo *looks* machine-parsable, so it should
> make people think about that.

The way I see it, this gives us more incentive to get server side log
templates working... and installed on svn.collab.net.

Sander

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: Support "Reviewed by" in addition to "Review by" in log messages

Posted by Peter Samuelson <pe...@p12n.org>.
[Garrett Rooney]
> If the script can be made to support the commonly used phrasing for
> these things then go for it, it's so much better than being overly
> anal about the exact wording people use.

OTOH, don't go overboard.  If you write some sort of natural language
parser that accepts almost anything, two years from now people will
look at the varied log messages and not realise that the format is
machine-parsed at all.  Then they'll outsmart the parser without even
noticing it.  The status quo *looks* machine-parsable, so it should
make people think about that.

Re: Support "Reviewed by" in addition to "Review by" in log messages

Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 2/9/06, Daniel Rall <dl...@collab.net> wrote:

> I've attached a patch which allows for both "Reviewed by" and "Review
> by", effectively making the former an alias for the latter.
>
> This implementation has the side-effect of generating HTML with log
> messages which don't exactly match what's in the repository, because
> canonical names (e.g. "Review by") are used in place of provided names
> (e.g. "Reviewed by").

FWIW, I think this is the proper way to solve such problems.  If the
script can be made to support the commonly used phrasing for these
things then go for it, it's so much better than being overly anal
about the exact wording people use.

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org


Re: Support "Reviewed by" in addition to "Review by" in log messages

Posted by Daniel Rall <dl...@collab.net>.
On Thu, 09 Feb 2006, Daniel Rall wrote:

> On Thu, 09 Feb 2006, Peter N. Lundblad wrote:
> 
> > On Wed, 8 Feb 2006, Daniel Rall wrote:
> > 
> > > On Wed, 08 Feb 2006, Peter N. Lundblad wrote:
> > >
> > > > On Tue, 7 Feb 2006, Justin Erenkrantz wrote:
> > > I wouldn't mind both being supported.  Peter's nitpick is important
> > > because the tools/dev/contribulyze.py script that Karl wrote depends
> > > upon an exact match:
> > >
> > > field_re = re.compile('^(Patch|Review|Suggested|Found) by:\s+(.*)')
> > >
> > +1.  Here, consistency isn't worth the noise. And, well, how many scripts
> > depend on this? :-)
> 
> I've attached a patch which allows for both "Reviewed by" and "Review
> by", effectively making the former an alias for the latter.
> 
> This implementation has the side-effect of generating HTML with log
> messages which don't exactly match what's in the repository, because
> canonical names (e.g. "Review by") are used in place of provided names
> (e.g. "Reviewed by").
...

Committed in r18424.
-- 

Daniel Rall