In this case, post-review should actually work. post-review with Subversion
shouid properly handle the case where you have new files that haven't been
committed yet that you want to put up for review. We did this all the time
with Review Board itself, before we switched over to Git.

Correct me if I'm wrong, but your script is useful when you want to put
existing committed code as a whole up for review, right? Such as when you're
doing a post-commit review for the first time of some code?

Christian

-- 
Christian Hammond - chip...@chipx86.com
Review Board - http://www.reviewboard.org
VMware, Inc. - http://www.vmware.com


On Thu, Aug 12, 2010 at 12:17 PM, Vesterbaek <vesterb...@gmail.com> wrote:

> post-review has the --revision-range argument, but this does not work
> (as far as I remember - or rather, last time I checked this some while
> ago), if you specify lower revision numbers than the initial. If you
> want do put whole files up for review, you'd normally want something
> ala:
> svn diff -r 0:HEAD
>
> but this yields:
> svn: Unable to find repository location for '' in revision 0
>
> To work around this, I've created a small python script 'post-review-
> initial', which manually creates the diff and passes that to post-
> review. You can pass a number of files or dirs as arguments. The files
> must be added (not necessarily committed) to svn. Not really sure if
> this is the best / most elegant solution, but it works :). Here goes
>
> #
>
> ---------------------------------------------------------------------------------------
> #!/usr/bin/python
> import sys
> import os
> import os.path
> import subprocess
> from optparse import OptionParser
>
> DUMMY_FILENAME = '__dummy_non_existing_file'
> TMP_FILENAME = '__post_review_initial_tmp_file.diff'
>
> def get_files(paths):
>    if not paths:
>        paths = ['.']
>    files = []
>    for p in paths:
>
>        if os.path.isdir(p):
>            dir = p
>        else:
>            dir = os.path.dirname(p)
>
>        _files = execute(['svn', 'list', '--recursive', p],
> split_lines=True)
>        _files = [os.path.join(dir, file) for file in _files]
>        _files = [os.path.abspath(f) for f in _files]
>
>        for f in _files:
>            if os.path.isfile(f):
>                files.append(f)
>
>    # return sorted list of files. convert to set and back to list to
> remove duplicates
>    return sort_and_remove_duplicates(files)
>
>
> def sort_and_remove_duplicates(a):
>    if a:
>        a.sort()
>        last = a[-1]
>        for i in range(len(a)-2, -1, -1):
>            if last == a[i]:
>                del a[i]
>            else:
>                last = a[i]
>        return a
>
>
> def create_file_diff(file):
>    diff = execute(['diff', '-u', '--new-file', DUMMY_FILENAME, file],
> split_lines=True, ignore_errors=True)
>
>    if not diff:
>        return ''
>
>    info = execute(['svn', 'info', file], split_lines=True)
>    for l in info:
>        if l.lower().startswith('url'):
>            url = l.split(':',1)[1].strip()
>        if l.lower().startswith('repository root'):
>            repos_root = l.split(':',1)[1].strip()
>        if l.lower().startswith('revision'):
>            rev = l.split(':',1)[1].strip()
>    if not url or not repos_root or not rev:
>        raise Exception('Failed to get svn info for ' + file)
>
>    url =  url.replace(repos_root, '')
>
>    diff.insert(0, 'Index: ' + url)
>    diff.insert(1,
> '===================================================================')
>    diff[2] = '--- ' + url + '\t(revision 0)'
>    diff[3] = '+++ ' + url + '\t(revision %s)' % (rev)
>
>    return '\n'.join(diff)
>
>
> def execute(command, env=None, split_lines=False, ignore_errors=False,
>            extra_ignore_errors=()):
>    if env:
>        env.update(os.environ)
>    else:
>        env = os.environ.copy()
>
>    env['LC_ALL'] = 'en_US.UTF-8'
>    env['LANGUAGE'] = 'en_US.UTF-8'
>
>    if sys.platform.startswith('win'):
>        p = subprocess.Popen(command,
>                             stdin=subprocess.PIPE,
>                             stdout=subprocess.PIPE,
>                             stderr=subprocess.STDOUT,
>                             shell=False,
>                             universal_newlines=True,
>                             env=env)
>    else:
>        p = subprocess.Popen(command,
>                             stdin=subprocess.PIPE,
>                             stdout=subprocess.PIPE,
>                             stderr=subprocess.STDOUT,
>                             shell=False,
>                             close_fds=True,
>                             universal_newlines=True,
>                             env=env)
>    if split_lines:
>        data = [line.rstrip('\n') for line in p.stdout.readlines()]
>    else:
>        data = p.stdout.read()
>    rc = p.wait()
>    if rc and not ignore_errors and rc not in extra_ignore_errors:
>        raise Exception('Failed to execute command: %s\n%s' %
> (command, data))
>
>    return data
>
> def parse_options(args):
>    parser = OptionParser(usage='%prog [-n] [dir1/file1] [dir2/
> file2] .... [post-review options]', version='%prog ' + '0.02')
>    parser.add_option('-n', '--output-diff', dest='output_diff_only',
> action='store_true', default=False, help='outputs a diff to the
> console and exits. Does not post')
>    # Add options after files to args list (used for post-review)
>    parser.disable_interspersed_args()
>    (options, args) = parser.parse_args(args)
>    return (options, args)
>
> def main(args):
>    (options,args) = parse_options(args)
>
>    files_dirs = []
>    post_review_args = []
>    for a in args:
>      if a.startswith('-'):
>        post_review_args.append(a)
>      else:
>        files_dirs.append(a)
>
>    files = get_files(files_dirs)
>    diff = '\n'.join([create_file_diff(file) for file in files])
>
>    if options.output_diff_only:
>      print diff
>    else:
>      f = open(TMP_FILENAME, 'w')
>      f.write(diff)
>      f.close()
>      execute(['python', os.path.join(os.path.dirname(__file__), 'post-
> review.py'), '--diff-filename='+TMP_FILENAME]+post_review_args)
>      # Cleanup
>      try:
>        os.unlink(TMP_FILENAME)
>      except:
>        pass
>
> if __name__ == '__main__':
>    main(sys.argv[1:])
> #
>
> ---------------------------------------------------------------------------------------
>
>  - Jeppe
>
> --
> Want to help the Review Board project? Donate today at
> http://www.reviewboard.org/donate/
> Happy user? Let us know at http://www.reviewboard.org/users/
> -~----------~----~----~----~------~----~------~--~---
> To unsubscribe from this group, send email to
> reviewboard+unsubscr...@googlegroups.com<reviewboard%2bunsubscr...@googlegroups.com>
> For more options, visit this group at
> http://groups.google.com/group/reviewboard?hl=en
>

-- 
Want to help the Review Board project? Donate today at 
http://www.reviewboard.org/donate/
Happy user? Let us know at http://www.reviewboard.org/users/
-~----------~----~----~----~------~----~------~--~---
To unsubscribe from this group, send email to 
reviewboard+unsubscr...@googlegroups.com
For more options, visit this group at 
http://groups.google.com/group/reviewboard?hl=en

Reply via email to