Add support to provide review_request_id as URL
Review Request #11107 — Created July 31, 2020 and submitted
If a developer (or CI) gets a review request to review he will get a whole URL because it was copy+pasted from browser. This change allows to easily use that URL to get the patch of given review request.
Provided an URL to rbt patch and saw that it extract the
review_request_id and the diff_id correctly and saw that
it correctly used the host.Also provided an URL as interdiff format
/4-5/
and
saw it was refused.
Description | From | Last Updated |
---|---|---|
How about making a reusable function in rbtools/utils/review_request.py (parse_review_request_url())? |
|
|
This should be utilizing the six module's compatibility imports. At the top of the file, in the correct import group … |
|
|
I've been mulling this over. I think rather than make an assumption about the last bit of a URL containing … |
|
|
E302 expected 2 blank lines, found 1 |
![]() |
|
F821 undefined name 'm' |
![]() |
|
F821 undefined name 'm' |
![]() |
|
F821 undefined name 'm' |
![]() |
|
W391 blank line at end of file |
![]() |
-
-
How about making a reusable function in
rbtools/utils/review_request.py
(parse_review_request_url()
)? -
rbtools/commands/patch.py (Diff revision 1) This should be utilizing the
six
module's compatibility imports. At the top of the file, in the correct import group (and organized alphabetically), you'd want:from django.utils.six.moves.urllib.parse import urlparse
-
rbtools/commands/patch.py (Diff revision 1) I've been mulling this over. I think rather than make an assumption about the last bit of a URL containing an ID, the aforementioned function for this should pattern match using a regex.
That would look something like:
re.compile(r'^(?<server_url>https?://.*/(?:/s/[^/]+/)?)r/(?<review_request_id>\d+)/?')
This would happen URLs with/without a subdomain, with/without a Local Site, and would capture even if passing in a diff URL.
This could potentially be extended to even look for a
diff/(\d+)/
, working as an alternative to providing--diff-revision
. Though that's certainly not something that needs to be implemented in this change.
Change Summary:
Switched to regex and added diff_id as group
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+29 -1) |
Checks run (1 failed, 1 succeeded)
flake8
Change Summary:
flake8
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+29 -1) |
Checks run (2 succeeded)
Testing Done: |
|
---|
Change Summary:
Allow
--diff-revision
if URL is provided.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 4 (+30 -1) |