Summary

Added a mercurial hook for posting review requests when pushing to central repo.

Review Request #7100 — Created March 21, 2015 and discarded Jan. 29, 2019, 1:40 p.m.

Information

Owner

halvorlu

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

A Mercurial hook to post to Review Board on push to a central server.

The hook was designed to make posting to Review Board easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
1. does not require the user to install RBTools locally
2. makes sure any changes pushed to the central server have been reviewed and approved
3. makes links of all references to tickets/bugs/issues, which rbt post doesn't do.
4. automatically finds the right review request to update if there are any new commits, based the commit ID and a date/author hash. This does not require the user to confirm anything, which rbt post (often) requires. This also allows the hook to recognize rebased/amended changesets, because the date/author hash is unchanged.

Testing Done

The hook has been tested both with some unit testing and partly in a production environment.
Unit testing also included testing communication with server, which revealed a few issues that have been fixed.
I wasn't sure how these unit tests should be included in the test suites for RBTools, so I've left them out for now.

Issues

Description	From	Last Updated
Col: 61 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 68 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 39 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 42 E251 unexpected spaces around keyword / parameter equals	reviewbot	March 21, 2015, 4:04 p.m.
Col: 44 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
'from rbtools.hooks.mercurial import *' used; unable to detect undefined names	reviewbot	April 1, 2015, 12:36 a.m.
Col: 59 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 63 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	March 21, 2015, 4:05 p.m.
Col: 42 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 51 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	April 1, 2015, 12:36 a.m.
Col: 61 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 68 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 39 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 42 E251 unexpected spaces around keyword / parameter equals	reviewbot	March 21, 2015, 4:05 p.m.
Col: 44 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:05 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	April 1, 2015, 12:36 a.m.
Col: 61 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 68 E226 missing whitespace around arithmetic operator	reviewbot	March 21, 2015, 4:04 p.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	April 1, 2015, 12:36 a.m.
'from rbtools.hooks.mercurial import *' used; unable to detect undefined names	reviewbot	April 1, 2015, 12:36 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 22 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 21 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 27 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 61 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 30 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 68 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 29 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 25 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 30 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:36 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	April 1, 2015, 12:37 a.m.
'from rbtools.hooks.mercurial import *' used; unable to detect undefined names	reviewbot	April 1, 2015, 12:37 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 22 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 21 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 27 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 23 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 61 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 30 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 68 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 29 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 25 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 30 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 26 W503 line break before binary operator	reviewbot	April 1, 2015, 12:37 a.m.
Col: 13 E128 continuation line under-indented for visual indent	reviewbot	April 1, 2015, 12:37 a.m.
Col: 59 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:35 a.m.
Col: 66 E226 missing whitespace around arithmetic operator	reviewbot	April 1, 2015, 12:35 a.m.
Col: 59 E226 missing whitespace around arithmetic operator	reviewbot	April 6, 2015, 4:23 a.m.
Col: 66 E226 missing whitespace around arithmetic operator	reviewbot	April 6, 2015, 4:23 a.m.
Col: 22 W503 line break before binary operator	reviewbot	April 6, 2015, 4:23 a.m.
Branch information would be helpful	misery	April 11, 2015, 10:45 a.m.
I think a new request should be in draft mode to allow the developer additional changes. Immediately public for updated …	misery	April 11, 2015, 10:45 a.m.
This could fail if there is no address or the admin marked his profile information as private.	misery	April 11, 2015, 10:45 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 12, 2015, 1:22 a.m.
Col: 1 E303 too many blank lines (3)	reviewbot	April 12, 2015, 1:22 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 12, 2015, 1:22 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 12, 2015, 1:22 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	April 12, 2015, 1:22 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 12, 2015, 1:22 a.m.
These imports should follow the PEP-8 standard (3 sections, first standard library, then third party, then "local" imports). We also …	david	April 15, 2015, 12:12 p.m.
We should avoid using str (which has different meanings in python 2 and 3) and instead use six.text_type	david	April 18, 2015, 3:44 a.m.
Please add a blank line before this.	david	April 18, 2015, 3:44 a.m.
You don't need the + here, since strings will auto concatenate. Can we also say "repository's" instead of "repo's"?	david	April 18, 2015, 3:44 a.m.
Please add a blank line before this.	david	April 18, 2015, 3:44 a.m.
This check should be not ticket_url	david	April 18, 2015, 3:44 a.m.
We generally use older-style format strings (`'%d changesets recieved' % len(changesets).	david	April 18, 2015, 3:44 a.m.
Please add blank lines around the conditional blocks here and below.	david	April 18, 2015, 3:44 a.m.
For multi-line docstrings, the closing """ should go on the next line.	david	April 18, 2015, 3:44 a.m.
You can skip the [] inside all and it will be a bit more efficient (using a generator expression instead …	david	April 18, 2015, 3:44 a.m.
Please use % formatting.	david	April 18, 2015, 3:44 a.m.
This looks like it's part of a different change.	david	April 18, 2015, 3:44 a.m.
""" on the next line.	david	April 18, 2015, 3:45 a.m.
The formatted-in string needs to be run through re.escape. Please also use %-formatting.	david	April 18, 2015, 3:45 a.m.
Please rearrange according to PEP-8.	david	April 18, 2015, 3:45 a.m.
Please put each key/value on its own line.	david	April 18, 2015, 3:45 a.m.
This should use six.text_type rather than str.	david	April 18, 2015, 3:45 a.m.
This should fit on one line.	david	April 18, 2015, 3:45 a.m.
Can you just put the format inline in the function call?	david	April 18, 2015, 3:45 a.m.
This could be a single statement: return max([ datetime_from_timestamp(diff.timestamp) for diff in revreq.get_diffs(only_fields='timestamp') ])	david	April 18, 2015, 3:45 a.m.
""" on the next line.	david	April 18, 2015, 3:45 a.m.
""" on the next line.	david	April 18, 2015, 3:45 a.m.
This should be "Review Board", not "ReviewBoard"	david	April 18, 2015, 3:45 a.m.
Same here.	david	April 18, 2015, 3:45 a.m.
And here.	david	April 18, 2015, 3:45 a.m.
And here.	david	April 18, 2015, 3:45 a.m.
This should all be a single format operation rather than a format and a bunch of concatenations. You can also …	david	April 18, 2015, 3:45 a.m.
Please import unicode_literals at the top of this file.	david	April 18, 2015, 3:45 a.m.
Docstrings for TestCase test methods shouldn't end in periods.	david	April 18, 2015, 3:45 a.m.
Docstrings for TestCase test methods shouldn't end in periods.	david	April 18, 2015, 3:45 a.m.
Docstrings for TestCase test methods shouldn't end in periods.	david	April 18, 2015, 3:45 a.m.
'itertools' imported but unused	reviewbot	April 18, 2015, 3:48 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:48 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:48 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:48 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:48 a.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 18, 2015, 3:48 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	April 18, 2015, 3:48 a.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 18, 2015, 3:48 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E402 module level import not at top of file	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 0	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 27 E127 continuation line over-indented for visual indent	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E303 too many blank lines (3)	reviewbot	April 18, 2015, 3:47 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 18, 2015, 3:47 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 18, 2015, 3:47 a.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 18, 2015, 3:47 a.m.
local variable 'repo_id2' is assigned to but never used	reviewbot	April 18, 2015, 3:47 a.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 18, 2015, 3:47 a.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 18, 2015, 3:47 a.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	April 18, 2015, 3:47 a.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	April 18, 2015, 3:47 a.m.
'hg' imported but unused	reviewbot	April 18, 2015, 3:47 a.m.
'ui' imported but unused	reviewbot	April 18, 2015, 3:47 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:47 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:47 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:47 a.m.
Col: 19 W503 line break before binary operator	reviewbot	April 18, 2015, 3:47 a.m.
'itertools' imported but unused	reviewbot	April 18, 2015, 3:50 a.m.
Code looks for "ticket_id_prefixes"	misery	April 27, 2015, 1:41 p.m.
repo_root cannot be found here on reviewboard because reviewboard will use a remote connection like http://mercurial/scm/hg/Test. "hg root" will return …	misery	April 27, 2015, 1:41 p.m.
Documentation uses "ticket_prefixes"	misery	April 27, 2015, 1:41 p.m.
The link works if we replace "re.escape(base_url)" with "base_url", otherwise the URL is broken and reviewboard shows a broken link. …	misery	April 27, 2015, 1:40 p.m.
Missing ticket_prefixes here :-) find_ticket_refs(new_plain_description, ticket_prefixes)	misery	April 27, 2015, 1:39 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:07 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:07 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:07 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 27, 2015, 2:07 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	April 27, 2015, 2:08 p.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E402 module level import not at top of file	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 0	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 27 E127 continuation line over-indented for visual indent	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E303 too many blank lines (3)	reviewbot	April 27, 2015, 2:08 p.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 29 E226 missing whitespace around arithmetic operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 5 E301 expected 1 blank line, found 0	reviewbot	April 27, 2015, 2:08 p.m.
Col: 1 E302 expected 2 blank lines, found 1	reviewbot	April 27, 2015, 2:08 p.m.
Col: 80 E501 line too long (80 > 79 characters)	reviewbot	April 27, 2015, 2:08 p.m.
Col: 80 E501 line too long (81 > 79 characters)	reviewbot	April 27, 2015, 2:08 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:08 p.m.
Col: 19 W503 line break before binary operator	reviewbot	April 27, 2015, 2:08 p.m.
reviewboardhook can be CONFIG_SECTION	misery	July 3, 2015, 5:56 a.m.
reviewboardhook can be CONFIG_SECTION	misery	July 3, 2015, 5:56 a.m.
This will fail if "line" is unicode. We fixed it with hasher.update(line.encode('utf-8')) UnicodeEncodeError: 'ascii' codec can't encode character u'\xfc' in …	misery	Aug. 25, 2015, 10:15 a.m.
It would be nice if the "get description" could be a separate function. So we can use it to check …	misery	Oct. 13, 2015, 6:11 a.m.
If you push multiple changesets in different branches this won't work with our workflow. Could you change it to this? …	misery	July 3, 2015, 5:56 a.m.
^1 will leads to a broken "base_commit_id" See: https://code.google.com/p/reviewboard/issues/detail?id=3915 I added this as a work-around to rbtools in "clients/mercurial.py" at …	misery	Oct. 13, 2015, 6:09 a.m.
Col: 17 W503 line break before binary operator	reviewbot	July 3, 2015, 5:56 a.m.
see above	misery	Oct. 13, 2015, 6:09 a.m.
see above	misery	Oct. 13, 2015, 6:09 a.m.
base_commit_id needs to be passed here, too. Because otherwise reviewboard will use "tip" as base and this will fail if …	misery	Oct. 13, 2015, 6:09 a.m.
We changed it to this since a rebase could lead to a useless "whitespace changed"-update: if len(line) > 0 and …	misery	Oct. 13, 2015, 6:09 a.m.

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/mercurial_push.py
    rbtools/hooks/tests.py

rbtools/hooks/common.py (Diff revision 1)

Show all issues

Col: 61
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 1)

Show all issues

Col: 68
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 1)

Show all issues

Col: 39
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 1)

Show all issues

Col: 42
 E251 unexpected spaces around keyword / parameter equals

rbtools/hooks/mercurial.py (Diff revision 1)

Show all issues

Col: 44
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial_push.py (Diff revision 1)

Show all issues

 'from rbtools.hooks.mercurial import *' used; unable to detect undefined names

rbtools/hooks/mercurial_push.py (Diff revision 1)

Show all issues

Col: 59
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial_push.py (Diff revision 1)

Show all issues

Col: 63
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial_push.py (Diff revision 1)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```

rbtools/hooks/mercurial_push.py (Diff revision 1)

Show all issues

Col: 42
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial_push.py (Diff revision 1)

Show all issues

Col: 51
 E226 missing whitespace around arithmetic operator

rbtools/hooks/tests.py (Diff revision 1)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Commit:

-	edc84363172857e90ed8a4d543a7a177ccff0d26
+	4da915e2ecb48a0695cfe1aef28fb332963e3aea

Diff:

Revision 2 (+507)

Show changes

	contrib/tools/mercurial-push
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/tests.py

Ignored Files:
    contrib/tools/mercurial-push



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/tests.py

Ignored Files:
    contrib/tools/mercurial-push

rbtools/hooks/common.py (Diff revision 2)

Show all issues

Col: 61
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 2)

Show all issues

Col: 68
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 2)

Show all issues

Col: 39
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 2)

Show all issues

Col: 42
 E251 unexpected spaces around keyword / parameter equals

rbtools/hooks/mercurial.py (Diff revision 2)

Show all issues

Col: 44
 E226 missing whitespace around arithmetic operator

rbtools/hooks/tests.py (Diff revision 2)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Commit:

-	4da915e2ecb48a0695cfe1aef28fb332963e3aea
+	6a2ed22daf7d1b3cf8f7ee541996770fffa5a286

Diff:

Revision 3 (+506)

Show changes

	contrib/tools/mercurial-push
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/tests.py

Ignored Files:
    contrib/tools/mercurial-push



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    rbtools/hooks/tests.py

Ignored Files:
    contrib/tools/mercurial-push

rbtools/hooks/common.py (Diff revision 3)

Show all issues

Col: 61
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 3)

Show all issues

Col: 68
 E226 missing whitespace around arithmetic operator

rbtools/hooks/tests.py (Diff revision 3)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Description:

		A Mercurial hook to post to ReviewBoard on push to a central server.

		The hook was designed to make posting to ReviewBoard easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

		The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
		1) does not require the user to install RBTools locally
~		2) makes sure any changes pushed to the central server have been reviewed and approved
	~	2) makes sure any changes pushed to the central server have been reviewed and approved
	+	3) makes links of all references to tickets/bugs/issues, which I don't think rbt post does?

Change Summary:

More options for logging into the ReviewBoard server such as API token or "submit-as".

Commit:

-	6a2ed22daf7d1b3cf8f7ee541996770fffa5a286
+	de5572195fb1e40a165d174022be1f26216fe218

Diff:

Revision 4 (+612)

Show changes

	setup.py
	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    setup.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    setup.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 4)

Show all issues

 'from rbtools.hooks.mercurial import *' used; unable to detect undefined names

contrib/tools/mercurial_push.py (Diff revision 4)
Show all issues
```
Col: 23
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 4)
Show all issues
```
Col: 22
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 4)
Show all issues
```
Col: 21
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 4)
Show all issues
```
Col: 27
 W503 line break before binary operator
```

rbtools/hooks/common.py (Diff revision 4)

Show all issues

Col: 23
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 4)

Show all issues

Col: 23
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 4)

Show all issues

Col: 61
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 4)

Show all issues

Col: 30
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 4)

Show all issues

Col: 68
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 29
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 25
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 30
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 4)
Show all issues
```
Col: 26
 W503 line break before binary operator
```

rbtools/hooks/tests.py (Diff revision 4)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Commit:

-	de5572195fb1e40a165d174022be1f26216fe218
+	810a40068dae1021f2c6e62c0f37bc0f776005e0

Diff:

Revision 5 (+611)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 5)

Show all issues

 'from rbtools.hooks.mercurial import *' used; unable to detect undefined names

contrib/tools/mercurial_push.py (Diff revision 5)
Show all issues
```
Col: 23
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 5)
Show all issues
```
Col: 22
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 5)
Show all issues
```
Col: 21
 W503 line break before binary operator
```
contrib/tools/mercurial_push.py (Diff revision 5)
Show all issues
```
Col: 27
 W503 line break before binary operator
```

rbtools/hooks/common.py (Diff revision 5)

Show all issues

Col: 23
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 5)

Show all issues

Col: 23
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 5)

Show all issues

Col: 61
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 5)

Show all issues

Col: 30
 W503 line break before binary operator

rbtools/hooks/common.py (Diff revision 5)

Show all issues

Col: 68
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 29
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 25
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 30
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```
rbtools/hooks/mercurial.py (Diff revision 5)
Show all issues
```
Col: 26
 W503 line break before binary operator
```

rbtools/hooks/tests.py (Diff revision 5)

Show all issues

Col: 13
 E128 continuation line under-indented for visual indent

Commit:

-	810a40068dae1021f2c6e62c0f37bc0f776005e0
+	8f7a030d32b693e373e64b54a5e1e9427fcdce98

Diff:

Revision 6 (+616)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

rbtools/hooks/common.py (Diff revision 6)

Show all issues

Col: 59
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 6)

Show all issues

Col: 66
 E226 missing whitespace around arithmetic operator

Description:

		A Mercurial hook to post to ReviewBoard on push to a central server.

		The hook was designed to make posting to ReviewBoard easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

		The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
~		1) does not require the user to install RBTools locally
~		2) makes sure any changes pushed to the central server have been reviewed and approved
~		3) makes links of all references to tickets/bugs/issues, which I don't think rbt post does?
	~	1. does not require the user to install RBTools locally
	~	2. makes sure any changes pushed to the central server have been reviewed and approved
	~	3. makes links of all references to tickets/bugs/issues, which I don't think rbt post does?
	+	4. automatically finds the right review request to update if there are any new commits, based on commit IDs. This does not require the user to confirm anything, which rbt post (often) requires.

Description:

		A Mercurial hook to post to ReviewBoard on push to a central server.

		The hook was designed to make posting to ReviewBoard easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

		The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
		1. does not require the user to install RBTools locally
		2. makes sure any changes pushed to the central server have been reviewed and approved
~		3. makes links of all references to tickets/bugs/issues, which I don't think rbt post does?
	~	3. makes links of all references to tickets/bugs/issues, which rbt post doesn't do.
		4. automatically finds the right review request to update if there are any new commits, based on commit IDs. This does not require the user to confirm anything, which rbt post (often) requires.

Description:

		A Mercurial hook to post to ReviewBoard on push to a central server.

		The hook was designed to make posting to ReviewBoard easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

		The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
		1. does not require the user to install RBTools locally
		2. makes sure any changes pushed to the central server have been reviewed and approved
		3. makes links of all references to tickets/bugs/issues, which rbt post doesn't do.
~		4. automatically finds the right review request to update if there are any new commits, based on commit IDs. This does not require the user to confirm anything, which rbt post (often) requires.
	~	4. automatically finds the right review request to update if there are any new commits, based the commit ID and a date/author hash. This does not require the user to confirm anything, which rbt post (often) requires. This also allows the hook to recognize rebased/amended changesets, because the date/author hash is unchanged.

Commit:

-	8f7a030d32b693e373e64b54a5e1e9427fcdce98
+	4bb44e1559bf1ed6f592b74569bfead445f50daa

Diff:

Revision 7 (+721)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

rbtools/hooks/common.py (Diff revision 7)

Show all issues

Col: 59
 E226 missing whitespace around arithmetic operator

rbtools/hooks/common.py (Diff revision 7)

Show all issues

Col: 66
 E226 missing whitespace around arithmetic operator

rbtools/hooks/mercurial.py (Diff revision 7)
Show all issues
```
Col: 22
 W503 line break before binary operator
```

Commit:

-	4bb44e1559bf1ed6f592b74569bfead445f50daa
+	cfde8d86b193e28e9e5d434bc14937ed614dcce4

Diff:

Revision 8 (+721)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

Diff:

Revision 9 (+721)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

Thank you for the hook and the implementation of my ideas. You got a mail! ;-)

rbtools/hooks/mercurial.py (Diff revision 9)
Show all issues
```
Branch information would be helpful
```

rbtools/hooks/mercurial.py (Diff revision 9)

Show all issues

I think a new request should be in draft mode to allow the developer additional changes. Immediately public for updated requests are ok. Maybe an option?

rbtools/hooks/mercurial.py (Diff revision 9)

Show all issues

This could fail if there is no address or the admin marked his profile information as private.

Change Summary:

Add option to publish draft (or not), add branch information, remove admin_email function.

Commit:

-	cfde8d86b193e28e9e5d434bc14937ed614dcce4
+	ccd37945eb7663a779e28ff1ca5d6f2ed8a7669d

Diff:

Revision 10 (+737)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

Commit:

-	ccd37945eb7663a779e28ff1ca5d6f2ed8a7669d
+	ae594336d78ce67fd07f7e47187302e426bcbe50

Diff:

Revision 11 (+1283)

Show changes

	contrib/tools/mercurial_push.py
	contrib/tools/test_reviewboardhook.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: PEP8 Style Checker
Processed Files:
    contrib/tools/test_reviewboardhook.py
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 1
 E303 too many blank lines (3)
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 11)
Show all issues
```
Col: 19
 W503 line break before binary operator
```

Commit:

-	ae594336d78ce67fd07f7e47187302e426bcbe50
+	17f95a982a7ba2d22a45729b017c5fbc07c5fdbf

Diff:

Revision 12 (+753 -9)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/clients/mercurial.py
	rbtools/commands/post.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py
	rbtools/utils/review_request.py

Tool: Pyflakes
Processed Files:
    contrib/tools/mercurial_push.py
    rbtools/utils/review_request.py
    rbtools/hooks/mercurial.py
    rbtools/commands/post.py
    rbtools/clients/mercurial.py
    rbtools/hooks/common.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    contrib/tools/mercurial_push.py
    rbtools/utils/review_request.py
    rbtools/hooks/mercurial.py
    rbtools/commands/post.py
    rbtools/clients/mercurial.py
    rbtools/hooks/common.py
    rbtools/hooks/tests.py

You have a mix of single-quoted and double-quoted strings throughout this change. Please go through and make sure you're using single quotes for everything unless the string itself has a single quote in it (for example, if there's a possesive).

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

These imports should follow the PEP-8 standard (3 sections, first standard library, then third party, then "local" imports). We also alphebetize imports when possible. As a result:

import itertools
import getpass
import logging

import rbtools.hooks.mercurial as hghook
from rbtools.api.errors import APIError
from rbtools.utils.filesystem import load_config

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

We should avoid using str (which has different meanings in python 2 and 3) and instead use six.text_type

contrib/tools/mercurial_push.py (Diff revision 12)
Show all issues
```
Please add a blank line before this.
```

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

You don't need the + here, since strings will auto concatenate. Can we also say "repository's" instead of "repo's"?

contrib/tools/mercurial_push.py (Diff revision 12)
Show all issues
```
Please add a blank line before this.
```
contrib/tools/mercurial_push.py (Diff revision 12)
Show all issues
```
This check should be not ticket_url
```

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

We generally use older-style format strings (`'%d changesets recieved' % len(changesets).

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

Please add blank lines around the conditional blocks here and below.

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

For multi-line docstrings, the closing """ should go on the next line.

contrib/tools/mercurial_push.py (Diff revision 12)

Show all issues

You can skip the [] inside all and it will be a bit more efficient (using a generator expression instead of a list comprehension)

contrib/tools/mercurial_push.py (Diff revision 12)
Show all issues
```
Please use % formatting.
```
rbtools/clients/mercurial.py (Diff revision 12)
Show all issues
```
This looks like it's part of a different change.
```
rbtools/hooks/common.py (Diff revision 12)
Show all issues
```
""" on the next line.
```

rbtools/hooks/common.py (Diff revision 12)

Show all issues

The formatted-in string needs to be run through re.escape. Please also use %-formatting.

rbtools/hooks/mercurial.py (Diff revision 12)

Show all issues

Please rearrange according to PEP-8.

rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
Please put each key/value on its own line.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
This should use six.text_type rather than str.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
This should fit on one line.
```

rbtools/hooks/mercurial.py (Diff revision 12)

Show all issues

Can you just put the format inline in the function call?

rbtools/hooks/mercurial.py (Diff revision 12)

Show all issues

This could be a single statement:

return max([
    datetime_from_timestamp(diff.timestamp)
    for diff in revreq.get_diffs(only_fields='timestamp')
])

rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
""" on the next line.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
""" on the next line.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
This should be "Review Board", not "ReviewBoard"
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
Same here.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
And here.
```
rbtools/hooks/mercurial.py (Diff revision 12)
Show all issues
```
And here.
```

rbtools/hooks/mercurial.py (Diff revision 12)

Show all issues

This should all be a single format operation rather than a format and a bunch of concatenations. You can also use python's built-in concatenation:

raise LoginError(
    'Could not open Review Board repository for path "%s". '
    'Please check with your server administrator to ensure '
    'you have the correct permissions.'
    % path)

rbtools/hooks/tests.py (Diff revision 12)

Show all issues

Please import unicode_literals at the top of this file.

rbtools/hooks/tests.py (Diff revision 12)

Show all issues

Docstrings for TestCase test methods shouldn't end in periods.

rbtools/hooks/tests.py (Diff revision 12)

Show all issues

Docstrings for TestCase test methods shouldn't end in periods.

rbtools/hooks/tests.py (Diff revision 12)

Show all issues

Docstrings for TestCase test methods shouldn't end in periods.

Change Summary:

Some formatting changes. Also added extra_data.diff_hash in order to know whether the diff has changed. This allows the hook to identify changesets that have been rebased, but not changed.

Commit:

-	17f95a982a7ba2d22a45729b017c5fbc07c5fdbf
+	ed8130f502dfe3a806b373ead5ce08d4ee9640e0

Diff:

Revision 13 (+1577)

Show changes

	contrib/tools/mercurial_push.py
	contrib/tools/test_reviewboardhook.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    contrib/tools/test_reviewboardhook.py
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    contrib/tools/test_reviewboardhook.py
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 13)
Show all issues
```
 'itertools' imported but unused
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E402 module level import not at top of file
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

contrib/tools/test_reviewboardhook.py (Diff revision 13)

Show all issues

Col: 27
 E127 continuation line over-indented for visual indent

contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E303 too many blank lines (3)
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
 local variable 'repo_id2' is assigned to but never used
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
 'hg' imported but unused
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
 'ui' imported but unused
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 13)
Show all issues
```
Col: 19
 W503 line break before binary operator
```

Change Summary:

Remove file that wasn't supposed to be included.

Diff:

Revision 14 (+824)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 14)
Show all issues
```
 'itertools' imported but unused
```

Commit:

-	ed8130f502dfe3a806b373ead5ce08d4ee9640e0
+	0a96916aeef020d9bd17780696b90bd3e85dc5bc

Diff:

Revision 15 (+823)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Description:

~		A Mercurial hook to post to ReviewBoard on push to a central server.
	~	A Mercurial hook to post to Review Board on push to a central server.

~		The hook was designed to make posting to ReviewBoard easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.
	~	The hook was designed to make posting to Review Board easy for new or inexperienced users. It allows user to post to ReviewBoard by using the ordinary "hg push", without any need to learn or install RBTools locally.

		The hook only allows the push if all commits have been approved in a review requests, otherwise it rejects it and creates a review request for the (new) commits. This is similar to "rbt post", but
		1. does not require the user to install RBTools locally
		2. makes sure any changes pushed to the central server have been reviewed and approved
		3. makes links of all references to tickets/bugs/issues, which rbt post doesn't do.
		4. automatically finds the right review request to update if there are any new commits, based the commit ID and a date/author hash. This does not require the user to confirm anything, which rbt post (often) requires. This also allows the hook to recognize rebased/amended changesets, because the date/author hash is unchanged.

Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 15)
Show all issues
```
Code looks for "ticket_id_prefixes"
```

contrib/tools/mercurial_push.py (Diff revision 15)

Show all issues

repo_root cannot be found here on reviewboard because reviewboard will use a remote connection like http://mercurial/scm/hg/Test. "hg root" will return "/opt/hg/Test".

contrib/tools/mercurial_push.py (Diff revision 15)
Show all issues
```
Documentation uses "ticket_prefixes"
```

Change Summary:

Find Review Board repository using name, not path.

Diff:

Revision 16 (+826)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

rbtools/hooks/mercurial.py (Diff revision 16)

Show all issues

Missing ticket_prefixes here :-)

find_ticket_refs(new_plain_description, ticket_prefixes)

halvorlu April 27, 2015, 1:41 p.m.

Thanks for taking the time to find these errors!

rbtools/hooks/common.py (Diff revision 16)

Show all issues

The link works if we replace "re.escape(base_url)" with "base_url", otherwise the URL is broken and reviewboard shows a broken link.

Example:
[reviewboardhook]
ticket_url=https://some.domain.de/jira/browse/
ticket_id_prefixes=EXAMPLEAPP-

halvorlu April 27, 2015, 1:41 p.m.

You are absolutely correct. re.escape escapes too much. I now replace only backslashes, which should suffice.

In the future I hope to be able to retrieve ticket_url directly from the Review Board repository configuration, but this is not supported by Review Board yet.

Diff:

Revision 17 (+1578)

Show changes

	contrib/tools/mercurial_push.py
	contrib/tools/test_reviewboardhook.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Tool: PEP8 Style Checker
Processed Files:
    contrib/tools/test_reviewboardhook.py
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/hooks/tests.py

contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E402 module level import not at top of file
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```

contrib/tools/test_reviewboardhook.py (Diff revision 17)

Show all issues

Col: 27
 E127 continuation line over-indented for visual indent

contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E303 too many blank lines (3)
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 29
 E226 missing whitespace around arithmetic operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 5
 E301 expected 1 blank line, found 0
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 1
 E302 expected 2 blank lines, found 1
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (80 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 80
 E501 line too long (81 > 79 characters)
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```
contrib/tools/test_reviewboardhook.py (Diff revision 17)
Show all issues
```
Col: 19
 W503 line break before binary operator
```

Commit:

-	0a96916aeef020d9bd17780696b90bd3e85dc5bc
+	38176300ef5e4009151406bab535712f6c22702c

Diff:

Revision 18 (+834 -70)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py
	rbtools/utils/review_request.py

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/utils/review_request.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/utils/review_request.py
    rbtools/hooks/tests.py

rbtools/hooks/mercurial.py (Diff revision 18)

Show all issues

If you push multiple changesets in different branches this won't work with our workflow. Could you change it to this?

branch = extcmd(['hg', 'id', '--branch', '-r', changesets[0]]).strip()

misery April 28, 2015, 3:49 a.m.

We added our workflow in the following code. It acts more like gerrit and the "one_review_per_changeset" option I told you. We rewrote the "push_review_hook_base" for that... I appreciate that you could merge our workflow as a separate method/option to your code. :-)

def push_review_hook_base(root, rbrepo, node, url, submitter):
    """Run the hook with given API root, Review Board repo and changeset.

    node is the commit ID of the first changeset in the push.
    url is the Review Board server URL.
    submitter is the user name of the user that is submitting.
    """
    ticket_url = get_ticket_url()
    ticket_prefixes = get_ticket_prefixes()
    changesets = hghook.list_of_incoming(node)
    parent = node + '^1'
    LOGGER.info('%d changesets received.', len(changesets))

    approvals = []
    revreqs = []
    for changeset in changesets:
        try:
            revreq = hghook.find_review_request(root, rbrepo, changeset)
            diff_hash = hghook.calc_diff_hash(hghook.calculate_diff(root, [changeset], None)['diff'])
            branch = hghook.extcmd(['hg', 'id', '--branch', '-r', changeset]).strip()

            if revreq.branch == branch and 'diff_hash' in revreq.extra_data and diff_hash == revreq.extra_data['diff_hash']:
                if hghook.approved_by_others(revreq):
                    LOGGER.info('Found approved review request (%d) for changeset: %s', revreq.id, changeset)
                    approvals.append(True)
                    revreqs.append(revreq)
                else:
                    LOGGER.info('Found unchanged review request (%d) for changeset: %s', revreq.id, changeset)
                    approvals.append(False)
            else:
                LOGGER.info('Updating review request (%d) for changeset: %s', revreq.id, changeset)
                update_and_publish(root, ticket_url, ticket_prefixes, [changeset], revreq, parent)
                approvals.append(False)

        except hghook.NotFoundError:
            if is_approved([changeset]):
                approvals.append(True)
            else:
                revreq = create(root, rbrepo, submitter, url, changeset)
                LOGGER.info('Creating review request (%d) for new changeset: %s', revreq.id, changeset)
                update_and_publish(root, ticket_url, ticket_prefixes, [changeset], revreq, parent)
                approvals.append(False)

    if all(approvals):
        for revreq in revreqs:
            LOGGER.info('Closing review request: ' + revreq.absolute_url)
            hghook.close_request(revreq)
        return hghook.HOOK_SUCCESS
    elif any(approvals):
        last_approved = approvals.index(False) - 1
        if last_approved > -1:
            LOGGER.info('If you want to push the already approved changes,')
            LOGGER.info('you can (probably) do this by executing')
            LOGGER.info('hg push -r %s', changesets[last_approved])

    return hghook.HOOK_FAILED

halvorlu April 29, 2015, 12:53 p.m.

Up until now, the hook was designed for changesets that were in the same branch and with only one head. If the changesets belong to different branches, I guess the best/easiest option is to have one review request per changeset, as you suggest. I'll try to merge this code with the existing one.

misery June 29, 2015, 9:32 a.m.

Did you found some time to merge it? :-)

contrib/tools/mercurial_push.py (Diff revision 18)
Show all issues
```
reviewboardhook can be CONFIG_SECTION
```
contrib/tools/mercurial_push.py (Diff revision 18)
Show all issues
```
reviewboardhook can be CONFIG_SECTION
```

If this is a separate method we can check this in our workflow

if revreq.branch == branch and 'diff_hash' in revreq.extra_data and diff_hash == revreq.extra_data['diff_hash'] and hghook.get_description([changeset], revreq, ticket_url, ticket_prefixes) == revreq.description:


instead of 

if revreq.branch == branch and 'diff_hash' in revreq.extra_data and diff_hash == revreq.extra_data['diff_hash']:

halvorlu April 29, 2015, 1:10 p.m.

Do you need to check the description in order to determine if the changeset should be approved? Is it because you want to enforce certain commit message standards? What I do in the current version is to run update_draft() for all changesets, which updates the description and the diff (if necessary). Then I approve the changeset if "Ship it"s have been issued after the last diff update.

misery May 5, 2015, 7:06 a.m.

Yes and no... we don't wait that another one "approves" it again but the commit message it reviewed as well like in gerrit. For now it should be updated just before it will be pushed to be equal with a the changeset.

rbtools/hooks/mercurial.py (Diff revision 18)

Show all issues

It would be nice if the "get description" could be a separate function. So we can use it to check if something is changed before we update the review request.

def get_description(changesets, revreq, ticket_url, ticket_prefixes):
    old_description = revreq.description
    new_plain_description = generate_description(changesets)
    linked_description = linkify_ticket_refs(new_plain_description,
                                             ticket_url, ticket_prefixes)
    return join_descriptions(old_description, linked_description)

misery April 28, 2015, 10:38 a.m.

Well... return value needs to be stripped here, otherwise the equal check will fail

   return join_descriptions(old_description, linked_description).strip()

rbtools/hooks/mercurial.py (Diff revision 18)

Show all issues

This will fail if "line" is unicode.

We fixed it with hasher.update(line.encode('utf-8'))
UnicodeEncodeError: 'ascii' codec can't encode character u'\xfc' in position 68: ordinal not in range(128)

halvorlu July 3, 2015, 5:30 a.m.

I thought line would be a byte string already? The diff should be a byte string, hence also line?

misery July 6, 2015, 11:33 a.m.

Well, I don't know. I just got this error if the diff contains special signs or an umlaut - so I added that as a work-around.

Change Summary:

Added option to review each changeset separately.

Diff:

Revision 19 (+922 -13)

Show changes

	contrib/tools/mercurial_push.py
	rbtools/clients/mercurial.py
	rbtools/hooks/common.py
	rbtools/hooks/mercurial.py
	rbtools/hooks/tests.py

Fix it!

Tool: Pyflakes
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/clients/mercurial.py
    rbtools/hooks/tests.py



Tool: PEP8 Style Checker
Processed Files:
    rbtools/hooks/common.py
    rbtools/hooks/mercurial.py
    contrib/tools/mercurial_push.py
    rbtools/clients/mercurial.py
    rbtools/hooks/tests.py

contrib/tools/mercurial_push.py (Diff revision 19)
Show all issues
```
Col: 17
 W503 line break before binary operator
```

Ping! Could some reviewer look into it to have it finally merged? We use this since 3-4 month without any problems.

Fix it!

contrib/tools/mercurial_push.py (Diff revision 19)

Show all issues

^1 will leads to a broken "base_commit_id"
See:

https://code.google.com/p/reviewboard/issues/detail?id=3915
I added this as a work-around to rbtools in "clients/mercurial.py" at "def diff":

        if "^" in base_commit_id:
            base_commit_id = self._execute(
                ['hg', 'log', '-r', base_commit_id, '-T {node}'],
                env=self._hg_env, results_unicode=False)

contrib/tools/mercurial_push.py (Diff revision 19)
Show all issues
```
see above
```
rbtools/hooks/mercurial.py (Diff revision 19)
Show all issues
```
see above
```

rbtools/hooks/mercurial.py (Diff revision 19)

Show all issues

base_commit_id needs to be passed here, too. Because otherwise reviewboard will use "tip" as base and this will fail if the changeset is based on a special branch.

diffs.upload_diff(diff_info['diff'],
                          base_commit_id=diff_info['base_commit_id'])

rbtools/hooks/mercurial.py (Diff revision 19)

Show all issues

We changed it to this since a rebase could lead to a useless "whitespace changed"-update:

if len(line) > 0 and (line[0] == b'+' or line[0] == b'-'):


Also this will use the embedded "date" in the patch that will be changed after a rebase.
Our workaround in "clients/mercurial.py" at "def diff":

diff_cmd = ['hg', 'diff', '--hidden', '--nodates']

Added a mercurial hook for posting review requests when pushing to central repo.

Information

Reviewers

Commit:

Diff:

Commit:

Diff:

Description:

Change Summary:

Commit:

Diff:

Commit:

Diff:

Commit:

Diff:

Description:

Description:

Description:

Commit:

Diff:

Commit:

Diff:

Diff:

Change Summary:

Commit:

Diff:

Commit:

Diff:

Commit:

Diff:

Change Summary:

Commit:

Diff:

Change Summary:

Diff:

Commit:

Diff:

Description:

Change Summary:

Diff:

Diff:

Commit:

Diff:

Change Summary:

Diff:

Status: Discarded