Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Delta Between Two Patch Sets: hgreview.py

Issue 29363768: Fixes 4662 - [hgreview] Add an option for suppressing the email after upload (Closed) Base URL: https://bitbucket.org/fhd/codingtools
Left Patch Set: Created Nov. 22, 2016, 3:01 p.m.
Right Patch Set: Use ui.status instead of print Created Nov. 23, 2016, 5:20 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
Left: Side by side diff | Download
Right: Side by side diff | Download
« no previous file with change/comment | « no previous file | no next file » | no next file with change/comment »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
LEFTRIGHT
1 import BaseHTTPServer 1 import BaseHTTPServer
2 import os 2 import os
3 import re 3 import re
4 import socket 4 import socket
5 import sys 5 import sys
6 import urllib 6 import urllib
7 7
8 from mercurial import cmdutil, error 8 from mercurial import cmdutil, error
9 9
10 SERVER = 'https://codereview.adblockplus.org' 10 SERVER = 'https://codereview.adblockplus.org'
11 UPLOADTOOL_URL = SERVER + '/static/upload.py' 11 UPLOADTOOL_URL = SERVER + '/static/upload.py'
12 12
13 cmdtable = {} 13 cmdtable = {}
14 command = cmdutil.command(cmdtable) 14 command = cmdutil.command(cmdtable)
15 15
16 16
17 @command('review', 17 @command('review',
18 [ 18 [
19 ('i', 'issue', '', 'If given, adds a patch set to this review, othe rwise create a new one.', 'ISSUE'), 19 ('i', 'issue', '', 'If given, adds a patch set to this review, othe rwise create a new one.', 'ISSUE'),
20 ('r', 'revision', '', 'Revision to diff against or a revision range to upload.', 'REV'), 20 ('r', 'revision', '', 'Revision to diff against or a revision range to upload.', 'REV'),
21 ('c', 'change', '', 'A single revision to upload.', 'REV'), 21 ('c', 'change', '', 'A single revision to upload.', 'REV'),
22 ('t', 'title', '', 'New review subject or new patch set title.', 'T ITLE'), 22 ('t', 'title', '', 'New review subject or new patch set title.', 'T ITLE'),
23 ('m', 'message', '', 'New review description or new patch set messa ge.', 'MESSAGE'), 23 ('m', 'message', '', 'New review description or new patch set messa ge.', 'MESSAGE'),
24 ('w', 'reviewers', '', 'Add reviewers (comma separated email addres ses or @adblockplus.org user names).', 'REVIEWERS'), 24 ('w', 'reviewers', '', 'Add reviewers (comma separated email addres ses or @adblockplus.org user names).', 'REVIEWERS'),
25 ('', 'cc', '', 'Add CC (comma separated email addresses or @adblock plus.org user names).', 'CC'), 25 ('', 'cc', '', 'Add CC (comma separated email addresses or @adblock plus.org user names).', 'CC'),
26 ('', 'private', None, 'Make the review restricted to reviewers and those CCed.'), 26 ('', 'private', None, 'Make the review restricted to reviewers and those CCed.'),
27 ('y', 'assume_yes', None, 'Assume that the answer to yes/no questio ns is \'yes\'.'), 27 ('y', 'assume_yes', None, 'Assume that the answer to yes/no questio ns is \'yes\'.'),
28 ('', 'print_diffs', None, 'Print full diffs.'), 28 ('', 'print_diffs', None, 'Print full diffs.'),
29 ('', 'no_mail', None, 'Don\'t send an email after uploading.', 'NO_ MAIL'), 29 ('', 'no_mail', None, 'Don\'t send an email after uploading.'),
Wladimir Palant 2016/11/22 15:39:35 This is a flag without any parameter - please drop
Felix Dahlke 2016/11/23 05:46:48 Done.
30 ], '[options] [path...]') 30 ], '[options] [path...]')
31 def review(ui, repo, *paths, **opts): 31 def review(ui, repo, *paths, **opts):
32 ''' 32 '''
33 Uploads a review to https://codereview.adblockplus.org/ or updates an 33 Uploads a review to https://codereview.adblockplus.org/ or updates an
34 existing review request. This will always send mails for new reviews, when 34 existing review request. This will always send mails for new reviews, when
35 updating a review mails will only be sent if a message is given. 35 updating a review mails will only be sent if a message is given.
36 ''' 36 '''
37 args = ['--oauth2', '--server', SERVER] 37 args = ['--oauth2', '--server', SERVER]
38 if ui.debugflag: 38 if ui.debugflag:
39 args.append('--noisy') 39 args.append('--noisy')
40 elif ui.verbose: 40 elif ui.verbose:
41 args.append('--verbose') 41 args.append('--verbose')
42 elif ui.quiet: 42 elif ui.quiet:
43 args.append('--quiet') 43 args.append('--quiet')
44 44
45 if (not opts.get("no_mail") and 45 if (not opts.get('no_mail') and
Vasily Kuznetsov 2016/11/22 15:29:35 According to our style guide this string should be
Felix Dahlke 2016/11/23 05:46:48 Whoops, something I'll have to get used to :P Done
46 (not opts.get('issue') or opts.get('message'))): 46 (not opts.get('issue') or opts.get('message'))):
Vasily Kuznetsov 2016/11/22 15:29:35 Flake8 also gives E129 on this line (suggesting to
Felix Dahlke 2016/11/23 05:46:48 Forgout about the whole flake8 thing. Running it o
Vasily Kuznetsov 2016/11/23 11:00:28 For me it seemed to work with an additional 4-spac
47 args.append('--send_mail') 47 args.append('--send_mail')
48 48
49 if opts.get('revision') and opts.get('change'): 49 if opts.get('revision') and opts.get('change'):
50 raise error.Abort('Ambiguous revision range, only one of --revision and --change can be specified.') 50 raise error.Abort('Ambiguous revision range, only one of --revision and --change can be specified.')
51 if opts.get('change'): 51 if opts.get('change'):
52 rev = repo[opts['change']] 52 rev = repo[opts['change']]
53 args.extend(['--rev', '{}:{}'.format(rev.parents()[0], rev)]) 53 args.extend(['--rev', '{}:{}'.format(rev.parents()[0], rev)])
54 elif opts.get('revision'): 54 elif opts.get('revision'):
55 args.extend(['--rev', opts['revision']]) 55 args.extend(['--rev', opts['revision']])
56 else: 56 else:
(...skipping 11 matching lines...) Expand all
68 if not opts.get('message'): 68 if not opts.get('message'):
69 opts['message'] = opts['title'] 69 opts['message'] = opts['title']
70 70
71 path = (ui.config('paths', 'default-push') 71 path = (ui.config('paths', 'default-push')
72 or ui.config('paths', 'default') 72 or ui.config('paths', 'default')
73 or '') 73 or '')
74 match = re.search(r'^(?:https://|ssh://hg@)(.*)', path) 74 match = re.search(r'^(?:https://|ssh://hg@)(.*)', path)
75 if match: 75 if match:
76 opts['base_url'] = 'https://' + match.group(1) 76 opts['base_url'] = 'https://' + match.group(1)
77 77
78 if not opts.get('no_mail'): 78 # Make sure there is at least one reviewer
79 # Make sure there is at least one reviewer 79 if not opts.get('reviewers'):
80 if not opts.get('reviewers'): 80 if opts.get('no_mail'):
81 opts['reviewers'] = ui.prompt('Reviewers (comma-separated): ', ' ') 81 ui.status('No reviewers specified, edit the review to add '
Vasily Kuznetsov 2016/11/22 15:29:35 This line is now 82 characters, so it probably wou
Wladimir Palant 2016/11/22 15:39:35 I'd still ask for reviewers here but accept empty
Felix Dahlke 2016/11/23 05:46:48 I personally prefer to add reviewers via the web i
Felix Dahlke 2016/11/23 05:46:48 I'm actually a pretty big fan of staying within th
Vasily Kuznetsov 2016/11/23 11:00:28 I share Felix's preference, probably because I'm b
Felix Dahlke 2016/11/23 11:36:04 Yeah same here, I'd really prefer it if we didn't
Wladimir Palant 2016/11/23 15:42:10 I guess that I could live with a warning.
Felix Dahlke 2016/11/23 16:06:03 Alright, gave that a go :) It's not incredibly int
82 if not opts['reviewers'].strip(): 82 'some.\n')
83 raise error.Abort('No reviewers given.') 83 else:
84 opts['reviewers'] = ui.prompt('Reviewers (comma-separated): ',
85 '')
86 if not opts['reviewers'].strip():
87 raise error.Abort('No reviewers given.')
84 88
85 for opt in ('reviewers', 'cc'): 89 for opt in ('reviewers', 'cc'):
86 if opts.get(opt): 90 if opts.get(opt):
87 users = [u if '@' in u else u + '@adblockplus.org' 91 users = [u if '@' in u else u + '@adblockplus.org'
88 for u in re.split(r'\s*,\s*', opts[opt])] 92 for u in re.split(r'\s*,\s*', opts[opt])]
89 opts[opt] = ','.join(users) 93 opts[opt] = ','.join(users)
90 94
91 for opt in ('issue', 'title', 'message', 'reviewers', 'cc', 'base_url'): 95 for opt in ('issue', 'title', 'message', 'reviewers', 'cc', 'base_url'):
92 if opts.get(opt, ''): 96 if opts.get(opt, ''):
93 args.extend(['--' + opt, opts[opt]]) 97 args.extend(['--' + opt, opts[opt]])
(...skipping 58 matching lines...) Expand 10 before | Expand all | Expand 10 after
152 </body> 156 </body>
153 </html> 157 </html>
154 ''' % port 158 ''' % port
155 159
156 # Run the upload tool 160 # Run the upload tool
157 issue, patchset = scope['RealMain']([upload_path] + args) 161 issue, patchset = scope['RealMain']([upload_path] + args)
158 162
159 # Wait for the page to check in and retrieve issue URL 163 # Wait for the page to check in and retrieve issue URL
160 if server: 164 if server:
161 server.handle_request() 165 server.handle_request()
LEFTRIGHT
« no previous file | no next file » | Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Toggle Comments ('s')

Powered by Google App Engine
This is Rietveld