-
-
reviewboard/webapi/resources/review_request.py (Diff revision 1) Col: 80 E501 line too long (95 > 79 characters)
Add a link to the latest diff revision in the review request resource
Review Request #8270 — Created July 6, 2016 and submitted
The review request resource now links to the latest diff (as
latest_diffin thelinkssection of the response) when the
associated review request has diffs.
Ran unit tests.
| Description | From | Last Updated |
|---|---|---|
|
Col: 80 E501 line too long (95 > 79 characters) |
 reviewbot
|
|
|
A useful method, but this is going to hit the database per item. Instead, I'd suggest updating get_queryset to do … |
 chipx86
|
|
|
We should also do a self.assertNumQueries with this. I'd expect the value to be (old value) + 1, with the … |
chipx86
|
|
|
Too many blank lines. |
chipx86
|
|
|
Col: 80 E501 line too long (95 > 79 characters) |
reviewbot
|
|
|
Just to verify, this isn't doing any extra queryies, because of the prefetch_related, right? Can we add a comment here … |
chipx86
|
|
|
Can we put request on its own line? In this particular case, it helps the argument from getting a bit … |
chipx86
|
|
|
Col: 80 E501 line too long (95 > 79 characters) |
reviewbot
|
-
-
reviewboard/webapi/resources/review_request.py (Diff revision 1) A useful method, but this is going to hit the database per item. Instead, I'd suggest updating
get_querysetto do a prefetch_related for DiffSet. (I wish we had the newerPrefetchstuff to limit fields and be smarter about queries, but alas). I think this would have to look something like:queryset = queryset.select_related('diffset_history').prefetch_related('diffsets')Then instead of
get_latest_diffset, we'd need to grab the last diff inreview_request.diffset_history.diffsets.all().Something like that.
-
reviewboard/webapi/tests/test_review_request.py (Diff revision 1) We should also do a
self.assertNumQuerieswith this. I'd expect the value to be (old value) + 1, with the above query. -
Change Summary:
Added a unit test to assert number of queries.
Rewrote db interaction to use fewer queries.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+123 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py -
reviewboard/webapi/resources/review_request.py (Diff revision 2) Col: 80 E501 line too long (95 > 79 characters)
-
-
reviewboard/webapi/resources/review_request.py (Diff revision 2) Just to verify, this isn't doing any extra queryies, because of the
prefetch_related, right? Can we add a comment here just mentioning that we already have this data and it's not a new query?Reason being that otherwise,
diffsets.latest().only('revision')would be a better query, if we didn't already have the data. -
reviewboard/webapi/resources/review_request.py (Diff revision 2) Can we put
requeston its own line? In this particular case, it helps the argument from getting a bit lost in the other noise.
Change Summary:
Address Christian's issues.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 3 (+126 -1) |
-
Tool: Pyflakes Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py -
reviewboard/webapi/resources/review_request.py (Diff revision 3) Col: 80 E501 line too long (95 > 79 characters)
Tool: Pyflakes Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py Tool: PEP8 Style Checker Processed Files: reviewboard/webapi/resources/review_request.py reviewboard/webapi/tests/test_review_request.py