22de90ef33ed

Add identifiers to ReviewChangeset's and ReviewSignoff's.

The identifier of a rcset/rsignoff is the filename it was saved as, which is
the hash of its contents.

This will be useful when we want to add replies to comments/signoffs.

CLI support for this feature has also been added (using --verbose and --debug
flags with 'hg review [-r REV]'), as well as some simple unit tests.
[view raw] [browse files]
author Steve Losh <steve@stevelosh.com>
date Sat, 27 Mar 2010 11:10:12 -0400
parents b1448ebe2909
children edb16fb75b47
branches/tags (none)
files review/api.py review/extension_ui.py review/messages.py review/tests/test_comment.py review/tests/test_signoff.py review/tests/util.py

Changes

--- a/review/api.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/api.py	Sat Mar 27 11:10:12 2010 -0400
@@ -305,6 +305,7 @@
                 data['lines'] = data['lines'].split(',')
                 data['lines'] = map(int, filter(None, data['lines']))
                 data['hgdate'] = util.parsedate(data['hgdate'])
+                data['identifier'] = _split_path_dammit(fn)[-1]
                 self.comments.append(ReviewComment(**data))
             self.comments.sort(key=operator.attrgetter('local_datetime'))
             
@@ -312,6 +313,7 @@
             for fn in signofffns:
                 data = _parse_data(self.repo['tip'][fn].data())
                 data['hgdate'] = util.parsedate(data['hgdate'])
+                data['identifier'] = _split_path_dammit(fn)[-1]
                 self.signoffs.append(ReviewSignoff(**data))
             self.signoffs.sort(key=operator.attrgetter('local_datetime'))
         else:
@@ -654,6 +656,7 @@
         comment.lines
         comment.message
         comment.local_datetime
+        comment.identifier
     
     Each item is a string, except for lines, hgdate, and local_datetime.
     
@@ -667,7 +670,7 @@
     was added.
     
     """
-    def __init__(self, author, hgdate, node, filename, lines, message, **extra):
+    def __init__(self, author, hgdate, node, filename, lines, message, identifier=None, **extra):
         """Initialize a ReviewComment.
         
         You shouldn't need to create these directly -- use a ReviewChangeset
@@ -688,6 +691,7 @@
         self.filename = filename
         self.lines = lines
         self.message = message
+        self.identifier = identifier
     
     def _render_data(self):
         """Render the data of this comment into a string for writing to disk.
@@ -728,6 +732,7 @@
         signoff.node
         signoff.opinion
         signoff.message
+        signoff.identifier
     
     Each item is a string, except for hgdate and local_datetime.
     
@@ -739,7 +744,7 @@
     was added.
     
     """
-    def __init__(self, author, hgdate, node, opinion, message, **extra):
+    def __init__(self, author, hgdate, node, opinion, message, identifier=None, **extra):
         """Initialize a ReviewSignoff.
         
         You shouldn't need to create these directly -- use a ReviewChangeset
@@ -760,6 +765,7 @@
         self.node = node
         self.opinion = opinion
         self.message = message
+        self.identifier = identifier
     
     def _render_data(self):
         """Render the data of this signoff into a string for writing to disk.
--- a/review/extension_ui.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/extension_ui.py	Sat Mar 27 11:10:12 2010 -0400
@@ -112,18 +112,35 @@
     )
     ui.write(messages.REVIEW_LOG_COMMENTS % (comment_count, author_count))
     
+    def _build_item_header(item, author_template, author_extra=None):
+        author = templatefilters.person(item.author)
+        author_args = (author,)
+        if author_extra:
+            author_args = author_args + author_extra
+        author_part = author_template % author_args
+        
+        age = templatefilters.age(item.hgdate)
+        age_part = messages.REVIEW_LOG_AGE % age
+        if ui.debugflag:
+            hash_part = messages.REVIEW_LOG_IDENTIFIER % item.identifier
+        elif ui.verbose:
+            hash_part = messages.REVIEW_LOG_IDENTIFIER % item.identifier[:12]
+        else:
+            hash_part = ''
+        detail_part = age_part + hash_part
+        
+        spacing = 80 - (len(author_part) + len(detail_part))
+        if spacing <= 0:
+            spacing = 1
+        spacing = ' ' * spacing
+        
+        return author_part + spacing + detail_part + '\n'
+        
+    
     def _print_comment(comment, before='', after=''):
         ui.write(before)
-        
-        author = templatefilters.person(comment.author)
-        author_part = messages.REVIEW_LOG_COMMENT_AUTHOR % author
+        ui.write(_build_item_header(comment, messages.REVIEW_LOG_COMMENT_AUTHOR))
         
-        age = templatefilters.age(comment.hgdate)
-        age_part = messages.REVIEW_LOG_AGE % age 
-        
-        spacing = ' ' * (80 - (len(author_part) + len(age_part)))
-        
-        ui.write(author_part + spacing + age_part + '\n')
         for line in comment.message.splitlines():
             ui.write(messages.REVIEW_LOG_COMMENT_LINE % line)
         
@@ -132,16 +149,9 @@
     def _print_signoff(signoff, before='', after=''):
         ui.write(before)
         
-        author = templatefilters.person(signoff.author)
         opinion = signoff.opinion or 'neutral'
-        author_part = messages.REVIEW_LOG_SIGNOFF_AUTHOR % (author, opinion)
+        ui.write(_build_item_header(signoff, messages.REVIEW_LOG_SIGNOFF_AUTHOR, (opinion,)))
         
-        age = templatefilters.age(signoff.hgdate)
-        age_part = messages.REVIEW_LOG_AGE % age
-        
-        spacing = ' ' * (80 - (len(author_part) + len(age_part)))
-        
-        ui.write(author_part + spacing + age_part + '\n')
         for line in signoff.message.splitlines():
             ui.write(messages.REVIEW_LOG_SIGNOFF_LINE % line)
         
@@ -342,6 +352,12 @@
     If any FILEs are given, only those diffs will be shown. If --quiet is used
     no diffs will be shown.
     
+    If --verbose is used the short identifier of each comment/signoff will
+    also be shown.
+    
+    If --debug is used the full identifier of each comment/signoff will also
+    be shown.
+    
     """)),
     (['review-comment'],
      ('Adding code review comments for changesets'),
--- a/review/messages.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/messages.py	Sat Mar 27 11:10:12 2010 -0400
@@ -94,6 +94,7 @@
 """
 
 REVIEW_LOG_AGE = """(%s)"""
+REVIEW_LOG_IDENTIFIER = """ (%s)"""
 REVIEW_LOG_FILE_HEADER = """changes in %s"""
 
 REVIEW_LOG_SKIPPED = """\
--- a/review/tests/test_comment.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/tests/test_comment.py	Sat Mar 27 11:10:12 2010 -0400
@@ -1,9 +1,10 @@
 from nose import *
 from util import *
-from .. import messages
+from .. import api, messages
 
 import os
 from mercurial import util as hgutil
+from mercurial.node import hex
 
 
 a1, a2 = (messages.REVIEW_LOG_COMMENT_AUTHOR % '|').split('|')
@@ -223,3 +224,37 @@
     assert a2 not in output
     assert messages.REVIEW_LOG_COMMENT_LINE % 'Test comment one.' not in output
 
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_comment_identifiers():
+    review(comment=True, message='Test comment one.', rev='1', files=['file_one'])
+    
+    rd = api.ReviewDatastore(get_ui(), get_sandbox_repo())
+    dsr = get_datastore_repo()
+    
+    comment = rd['1'].comments[0]
+    
+    identifier = comment.identifier
+    short_identifier = identifier[:12]
+    
+    comment_filename = api._split_path_dammit(dsr['tip'].files()[0])[-1]
+    comment_cset = hex(dsr['tip'].node())
+    
+    assert identifier == comment_filename
+    assert identifier != comment_cset
+    
+    verbose_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier[:12]
+    debug_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier
+    
+    normal_output = review(rev='1')
+    assert verbose_identifier not in normal_output
+    assert debug_identifier not in normal_output
+    
+    verbose_output = review(rev='1', verbose=True)
+    assert verbose_identifier in verbose_output
+    assert debug_identifier not in verbose_output
+    
+    debug_output = review(rev='1', debug=True)
+    assert verbose_identifier not in debug_output
+    assert debug_identifier in debug_output
+
--- a/review/tests/test_signoff.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/tests/test_signoff.py	Sat Mar 27 11:10:12 2010 -0400
@@ -4,6 +4,7 @@
 
 import os
 from mercurial import util as hgutil
+from mercurial.node import hex
 
 
 s1, s2 = (messages.REVIEW_LOG_SIGNOFF_AUTHOR % ('|', 'neutral')).split('|')
@@ -91,3 +92,37 @@
     assert sn1 in output
     assert messages.REVIEW_LOG_SIGNOFF_LINE % 'Test signoff one.' in output
 
+
+@with_setup(setup_reviewed_sandbox, teardown_sandbox)
+def test_signoff_identifiers():
+    review(signoff=True, message='Test signoff one.', rev='0')
+    
+    rd = api.ReviewDatastore(get_ui(), get_sandbox_repo())
+    dsr = get_datastore_repo()
+    
+    signoff = rd['0'].signoffs[0]
+    
+    identifier = signoff.identifier
+    short_identifier = identifier[:12]
+    
+    signoff_filename = api._split_path_dammit(dsr['tip'].files()[0])[-1]
+    signoff_cset = hex(dsr['tip'].node())
+    
+    assert identifier == signoff_filename
+    assert identifier != signoff_cset
+    
+    verbose_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier[:12]
+    debug_identifier = messages.REVIEW_LOG_IDENTIFIER % identifier
+    
+    normal_output = review(rev='0')
+    assert verbose_identifier not in normal_output
+    assert debug_identifier not in normal_output
+    
+    verbose_output = review(rev='0', verbose=True)
+    assert verbose_identifier in verbose_output
+    assert debug_identifier not in verbose_output
+    
+    debug_output = review(rev='0', debug=True)
+    assert verbose_identifier not in debug_output
+    assert debug_identifier in debug_output
+
--- a/review/tests/util.py	Fri Mar 05 07:05:44 2010 -0500
+++ b/review/tests/util.py	Sat Mar 27 11:10:12 2010 -0400
@@ -9,12 +9,16 @@
 _ui = ui.ui()
 def review(init=False, comment=False, signoff=False, yes=False, no=False,
     force=False, message='', rev='.', remote_path='', lines='', files=None,
-    unified='5', web=False):
+    unified='5', web=False, verbose=False, debug=False):
     
     if not files:
         files = []
     
     _ui.pushbuffer()
+    if debug:
+        _ui.debugflag = True
+    elif verbose:
+        _ui.verbose = True
     extension_ui.review(
         _ui, get_sandbox_repo(), *files,
         **dict(
@@ -23,6 +27,7 @@
             lines=lines, unified=unified, web=web
         )
     )
+    _ui.verbose, _ui.debugflag = False, False
     output = _ui.popbuffer()
     
     print output
@@ -88,5 +93,9 @@
 def clone_sandbox_repo():
     hg.clone(cmdutil.remoteui(_ui, {}), sandbox_repo_path, sandbox_clone_path)
 
-def get_datastore_repo(path):
+def get_datastore_repo(path=api.DEFAULT_DATASTORE_DIRNAME):
     return hg.repository(_ui, path)
+
+def get_ui():
+    return _ui
+