flake8
failed.
JSHint
passed.
flake8
-
reviewboard/reviews/ui/base.py (Diff revision 1) Show all issues -
WIP: Dynamic Review UI Rendering
Review Request #10758 — Created Oct. 20, 2019 and updated
| Information | |
|---|---|
| nicolelisa | |
| Review Board | |
| master | |
| 10713 | |
| Reviewers | |
| reviewboard, students | |
Overview:
Add in functionality to allow review UIs to provide a list of (relative) URLs they want to register, along with the methods that will be handling requests to those URLs.
Task specifications can be found here
This task has been separated into 4 subtasks:
1. Base(1): Review UI URLs - Adding the ability for a ReviewUI subclass to provide URLs they want to make available. Assigned to Nicole.
2. Text(1): Built-In Render URLs - Adding a standard_render/URL forTextBasedReviewUI. This would ultimately take atype=argument specifying (sourceorrendered), which could be passed by the caller. Assigned to Nicole.
3. Text(2): Rendering Options Template Block - Updatereviewboard/templates/reviews/ui/text.htmltemplate to provide a{% block "render_options" %}that subclasses of the template can use to add new UI. This would be blank by default, meaning it's entirely optional, and can be used only by the review UIs that need it. Assigned to Adil.
4. Text(3): Render Requests in JavaScript - WrapRB.apiCallin a new method to provide Review UIs with the ability to make requests for custom rendering options and backend rendering implementations. Assigned to Adil
Adil's Testing for Tasks 3 and 4:
- Ran JavaScript unit tests.
- Tested manually to make sure that the element to update actually gets
updated by using a mock endpoint.- Tested manually to make sure comments are correctly reattached after
a request is finished.
| Summary | Author |
|---|---|
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa | |
| nicolelisa |
| Description | From | Last Updated |
|---|---|---|
|
E402 module level import not at top of file |
 reviewbot
|
|
|
E501 line too long (82 > 79 characters) |
reviewbot
|
|
|
F401 'django.conf.urls.include' imported but unused |
reviewbot
|
|
|
E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
E123 closing bracket does not match indentation of opening bracket's line |
reviewbot
|
|
|
F401 'django.core.urlresolvers.reverse_lazy' imported but unused |
reviewbot
|
|
|
F401 'django.core.urlresolvers.reverse' imported but unused |
reviewbot
|
|
|
F401 'django.http.Http404' imported but unused |
reviewbot
|
|
|
F401 'django.core.urlresolvers.resolve' imported but unused |
reviewbot
|
|
|
F401 'django.db.models.Q' imported but unused |
reviewbot
|
|
|
F401 'django.http.Http404' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseForbidden' imported but unused |
reviewbot
|
|
|
F401 'django.core.urlresolvers.resolve' imported but unused |
reviewbot
|
|
|
F401 'django.db.models.Q' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_object_or_404' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused |
reviewbot
|
|
|
This shouldn't be staticmethod and should be the get method, ie def get(self, ...): |
 brennie
|
|
|
Once ReviewFileAttachmentViewMixin is not using staticmethods, this should just be self.get_attachments, etc. |
brennie
|
|
|
Here too |
brennie
|
|
|
F841 local variable 'review_ui' is assigned to but never used |
reviewbot
|
|
|
W293 blank line contains whitespace |
reviewbot
|
|
|
This should be TextBasedReviewUIView.as_view() |
brennie
|
|
|
These should not be staticmethods; these should be regular methods instead. |
brennie
|
|
|
These should not be staticmethods; these should be regular methods instead. |
brennie
|
|
|
F841 local variable 'review_ui' is assigned to but never used |
reviewbot
|
|
|
E303 too many blank lines (2) |
reviewbot
|
|
|
In addition to these new tests, you should also have tests that assert that unregistering the review UI has the … |
 amalik2
|
|
|
This import statement should probably be moved to the top of this file for consistency. |
amalik2
|
|
|
Is there any way this can be done without modifying a global variable in another file? If there isn't, then … |
amalik2
|
|
|
This import statement should probably be moved to the top of this file for consistency. |
amalik2
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'djblets.util.decorators.augment_method_from' imported but unused |
reviewbot
|
|
|
F401 'djblets.webapi.resources.WebAPIResource' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.models.ReviewRequest' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused |
reviewbot
|
|
|
F401 'django.views.generic.base.ContextMixin' imported but unused |
reviewbot
|
|
|
F401 'django.views.generic.base.TemplateView' imported but unused |
reviewbot
|
|
|
F401 'django.views.generic.base.RedirectView' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.webapi.decorators.webapi_check_local_site' imported but unused |
reviewbot
|
|
|
You should write some tests to make sure that this method works as expected |
amalik2
|
|
|
E501 line too long (81 > 79 characters) |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
Is kwargs meant to be used by individual review UIs in the future? I think all of the query parameter … |
amalik2
|
|
|
Just curious about this, how can specific review UIs like the JSON or XML ones render text in different ways … |
amalik2
|
|
|
E501 line too long (84 > 79 characters) |
reviewbot
|
|
|
F841 local variable 'review_ui' is assigned to but never used |
reviewbot
|
|
|
E303 too many blank lines (2) |
reviewbot
|
|
|
F401 'django.db.models.Q' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_object_or_404' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F401 'django.http.Http404' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.render_to_response' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused |
reviewbot
|
|
|
F401 'django.views.generic.base.View' imported but unused |
reviewbot
|
|
|
F811 redefinition of unused 'ReviewFileAttachmentViewMixin' from line 76 |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F841 local variable 'is_enabled_for' is assigned to but never used |
reviewbot
|
|
|
F401 'django.http.HttpResponseNotFound' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseBadRequest' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_list_or_404' imported but unused |
reviewbot
|
|
|
E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
E116 unexpected indentation (comment) |
reviewbot
|
|
|
W391 blank line at end of file |
reviewbot
|
|
|
F401 'django.db.models.Q' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_object_or_404' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.http.Http404' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.render_to_response' imported but unused |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseBadRequest' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseNotFound' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_list_or_404' imported but unused |
reviewbot
|
|
|
E116 unexpected indentation (comment) |
reviewbot
|
|
|
E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
W391 blank line at end of file |
reviewbot
|
|
|
F401 'django.db.models.Q' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_object_or_404' imported but unused |
reviewbot
|
|
|
F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.http.Http404' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.render_to_response' imported but unused |
reviewbot
|
|
|
E126 continuation line over-indented for hanging indent |
reviewbot
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseBadRequest' imported but unused |
reviewbot
|
|
|
F401 'django.http.HttpResponseNotFound' imported but unused |
reviewbot
|
|
|
F401 'django.shortcuts.get_list_or_404' imported but unused |
reviewbot
|
|
|
E116 unexpected indentation (comment) |
reviewbot
|
|
|
E501 line too long (86 > 79 characters) |
reviewbot
|
|
|
W391 blank line at end of file |
reviewbot
|
|
|
E302 expected 2 blank lines, found 1 |
reviewbot
|
|
|
E302 expected 2 blank lines, found 0 |
reviewbot
|
|
|
E265 block comment should start with '# ' |
reviewbot
|
|
|
This should be removed, I've removed it from my AJAX function review request now as well |
amalik2
|
|
|
This should be removed, I've removed it from my AJAX function review request now as well |
amalik2
|
|
|
This should be removed, I've removed it from my AJAX function review request now as well |
amalik2
|
|
|
This should be removed, I've removed it from my AJAX function review request now as well |
amalik2
|
|
|
All changes to this file should be reverted since they were just for testing purposes |
amalik2
|
|
|
F401 'django.http.HttpResponse' imported but unused |
reviewbot
|
|
|
F401 'django.utils.safestring.mark_safe' imported but unused |
reviewbot
|
|
|
F401 'djblets.util.compat.django.template.loader.render_to_string' imported but unused |
reviewbot
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This isn't too important but technically this should be an HTTP 400 error. 404 indicates the resource was not found, … |
amalik2
|
|
|
This will need to be slightly modified to handle diff chunks, since those aren't stored using context['rendered_lines'] or context['text_lines'] |
amalik2
|
|
|
In my XML review request I had to set context['lines'] to the result of the generate_render() function to bypass the … |
amalik2
|
|
|
This should be context['lines'] = context['text_lines'] |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This should either be implemented and used in some way or removed |
amalik2
|
|
|
Maybe a different name for this key would be more suitable? It may not be that intuitive to others what … |
amalik2
|
|
|
This comment should be removed |
amalik2
|
|
|
This comment should be removed |
amalik2
|
|
|
This isn't important for this review request but in my AJAX function review request, there was a check for whether … |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
The docstring should be modified to include the 'renderType' parameter since you are using it |
amalik2
|
|
|
This should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
|
|
This test code should be removed |
amalik2
|
Commits: |
|
||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+290 -22) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
reviewboard/reviews/tests/test_file_attachment_review_ui.py (Diff revision 2) F401 'django.conf.urls.include' imported but unused
-
reviewboard/reviews/tests/test_file_attachment_review_ui.py (Diff revision 2) E123 closing bracket does not match indentation of opening bracket's line
-
reviewboard/reviews/tests/test_file_attachment_review_ui.py (Diff revision 2) E123 closing bracket does not match indentation of opening bracket's line
-
reviewboard/reviews/tests/test_file_attachment_review_ui.py (Diff revision 2) E123 closing bracket does not match indentation of opening bracket's line
-
reviewboard/reviews/ui/base.py (Diff revision 2) F401 'django.core.urlresolvers.reverse_lazy' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 2) F401 'django.core.urlresolvers.reverse' imported but unused
Commits: |
|
||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+293 -27) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Commits: |
|
||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+375 -39) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
-
reviewboard/reviews/ui/text.py (Diff revision 4) F401 'django.core.urlresolvers.resolve' imported but unused
-
Commits: |
|
|||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 5 (+563 -119) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
-
reviewboard/reviews/ui/text.py (Diff revision 5) F401 'django.http.HttpResponseForbidden' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 5) F401 'django.core.urlresolvers.resolve' imported but unused
-
-
reviewboard/reviews/ui/text.py (Diff revision 5) F401 'django.shortcuts.get_object_or_404' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 5) F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 5) F841 local variable 'review_ui' is assigned to but never used
-
-
reviewboard/reviews/views.py (Diff revision 5) F841 local variable 'review_ui' is assigned to but never used
-
-
-
reviewboard/reviews/ui/text.py (Diff revision 5) This shouldn't be staticmethod and should be the get method, ie
def get(self, ...): -
reviewboard/reviews/ui/text.py (Diff revision 5) Once
ReviewFileAttachmentViewMixinis not using staticmethods, this should just beself.get_attachments, etc. -
-
-
reviewboard/reviews/views.py (Diff revision 5) These should not be staticmethods; these should be regular methods instead.
-
reviewboard/reviews/views.py (Diff revision 5) These should not be staticmethods; these should be regular methods instead.
Commits: |
|
|||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 6 (+605 -119) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'django.http.HttpResponse' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'djblets.util.decorators.augment_method_from' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'djblets.webapi.resources.WebAPIResource' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'reviewboard.reviews.models.ReviewRequest' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'django.views.generic.base.ContextMixin' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'django.views.generic.base.TemplateView' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'django.views.generic.base.RedirectView' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 6) F401 'reviewboard.webapi.decorators.webapi_check_local_site' imported but unused
-
-
reviewboard/reviews/ui/text.py (Diff revision 6) E126 continuation line over-indented for hanging indent
-
-
reviewboard/reviews/views.py (Diff revision 6) F841 local variable 'review_ui' is assigned to but never used
-
-
-
reviewboard/reviews/tests/test_file_attachment_review_ui.py (Diff revision 6) In addition to these new tests, you should also have tests that assert that unregistering the review UI has the expected behaviour.
-
reviewboard/reviews/ui/base.py (Diff revision 6) This import statement should probably be moved to the top of this file for consistency.
-
reviewboard/reviews/ui/base.py (Diff revision 6) Is there any way this can be done without modifying a global variable in another file? If there isn't, then for encapsulation purposes it may be better to have a method in the urls.py file that takes in a urlpatterns argument, and appends it to the dyanmic URL resolver. Something like this:
def add_dynamic_review_ui_urls(urlpatterns):
.... -
reviewboard/reviews/ui/base.py (Diff revision 6) This import statement should probably be moved to the top of this file for consistency.
-
reviewboard/reviews/ui/text.py (Diff revision 6) You should write some tests to make sure that this method works as expected
-
reviewboard/reviews/ui/text.py (Diff revision 6) Is kwargs meant to be used by individual review UIs in the future? I think all of the query parameter values are available in the request object, which already gets passed to the review UI
-
reviewboard/reviews/ui/text.py (Diff revision 6) Just curious about this, how can specific review UIs like the JSON or XML ones render text in different ways based on the query parameter values? Where should the code that accesses them actually be (i.e. would it be in get_render_lines? If so, wouldn't that function need to be modified to take in a request argument?)
Commits: |
|
|||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 7 (+1241 -631) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
-
reviewboard/reviews/ui/base.py (Diff revision 7) F401 'django.shortcuts.get_object_or_404' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 7) F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 7) E126 continuation line over-indented for hanging indent
-
-
reviewboard/reviews/ui/text.py (Diff revision 7) F401 'django.shortcuts.render_to_response' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 7) F401 'reviewboard.reviews.views.ReviewFileAttachmentView' imported but unused
-
reviewboard/reviews/ui/text.py (Diff revision 7) F401 'django.views.generic.base.View' imported but unused
-
reviewboard/reviews/views.py (Diff revision 7) F811 redefinition of unused 'ReviewFileAttachmentViewMixin' from line 76
-
reviewboard/reviews/views.py (Diff revision 7) E126 continuation line over-indented for hanging indent
-
reviewboard/reviews/views.py (Diff revision 7) F841 local variable 'is_enabled_for' is assigned to but never used
-
reviewboard/reviews/views_mixins.py (Diff revision 7) F401 'django.http.HttpResponseNotFound' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 7) F401 'django.http.HttpResponse' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 7) F401 'django.http.HttpResponseBadRequest' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 7) F401 'django.shortcuts.get_list_or_404' imported but unused
-
-
-
Commits: |
|
|||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 8 (+1269 -649) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
-
reviewboard/reviews/ui/base.py (Diff revision 8) F401 'django.shortcuts.get_object_or_404' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 8) F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 8) E126 continuation line over-indented for hanging indent
-
reviewboard/reviews/ui/text.py (Diff revision 8) F401 'django.http.HttpResponse' imported but unused
-
-
reviewboard/reviews/ui/text.py (Diff revision 8) F401 'django.shortcuts.render_to_response' imported but unused
-
reviewboard/reviews/views.py (Diff revision 8) E126 continuation line over-indented for hanging indent
-
reviewboard/reviews/views_mixins.py (Diff revision 8) F401 'django.http.HttpResponse' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 8) F401 'django.http.HttpResponseBadRequest' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 8) F401 'django.http.HttpResponseNotFound' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 8) F401 'django.shortcuts.get_list_or_404' imported but unused
-
-
-
Commits: |
|
||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 9 (+1239 -679) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
-
reviewboard/reviews/ui/base.py (Diff revision 9) F401 'django.shortcuts.get_object_or_404' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 9) F401 'reviewboard.reviews.views_mixins.ReviewRequestViewMixin' imported but unused
-
reviewboard/reviews/ui/base.py (Diff revision 9) E126 continuation line over-indented for hanging indent
-
reviewboard/reviews/ui/text.py (Diff revision 9) F401 'django.http.HttpResponse' imported but unused
-
-
reviewboard/reviews/ui/text.py (Diff revision 9) F401 'django.shortcuts.render_to_response' imported but unused
-
reviewboard/reviews/views.py (Diff revision 9) E126 continuation line over-indented for hanging indent
-
reviewboard/reviews/views_mixins.py (Diff revision 9) F401 'django.http.HttpResponse' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 9) F401 'django.http.HttpResponseBadRequest' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 9) F401 'django.http.HttpResponseNotFound' imported but unused
-
reviewboard/reviews/views_mixins.py (Diff revision 9) F401 'django.shortcuts.get_list_or_404' imported but unused
-
-
-
Commits: |
|
||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 10 (+1240 -698) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 11 (+1708 -706) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
Summary: |
|
||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
||||||||||||||||||||||||||||||
Testing Done: |
|
Description: |
|
||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
Commits: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 12 (+2092 -764) |
Checks run (1 failed, 1 succeeded)
flake8
failed.
JSHint
passed.
flake8
-
reviewboard/reviews/ui/markdownui.py (Diff revision 12) F401 'django.http.HttpResponse' imported but unused
-
reviewboard/reviews/ui/markdownui.py (Diff revision 12) F401 'django.utils.safestring.mark_safe' imported but unused
-
reviewboard/reviews/ui/markdownui.py (Diff revision 12) F401 'djblets.util.compat.django.template.loader.render_to_string' imported but unused
-
-
reviewboard/admin/urls.py (Diff revision 12) This should be removed, I've removed it from my AJAX function review request now as well
-
reviewboard/admin/urls.py (Diff revision 12) This should be removed, I've removed it from my AJAX function review request now as well
-
reviewboard/admin/urls.py (Diff revision 12) This should be removed, I've removed it from my AJAX function review request now as well
-
reviewboard/admin/urls.py (Diff revision 12) This should be removed, I've removed it from my AJAX function review request now as well
-
reviewboard/reviews/ui/markdownui.py (Diff revision 12) All changes to this file should be reverted since they were just for testing purposes
-
-
-
reviewboard/reviews/ui/text.py (Diff revision 12) This isn't too important but technically this should be an HTTP 400 error. 404 indicates the resource was not found, while 400 indicates that the client sent a bad request (in this case, an invalid render_type value counts as a bad request)
-
reviewboard/reviews/ui/text.py (Diff revision 12) This will need to be slightly modified to handle diff chunks, since those aren't stored using context['rendered_lines'] or context['text_lines']
-
reviewboard/reviews/ui/text.py (Diff revision 12) In my XML review request I had to set context['lines'] to the result of the generate_render() function to bypass the caching that occurs in get_rendered_lines(), I think you'll have to do that here as well
-
reviewboard/reviews/ui/text.py (Diff revision 12) This should be context['lines'] = context['text_lines']
-
-
reviewboard/reviews/ui/text.py (Diff revision 12) This should either be implemented and used in some way or removed
-
reviewboard/reviews/ui/text.py (Diff revision 12) Maybe a different name for this key would be more suitable? It may not be that intuitive to others what 'reviewUIId' actually means. 'reviewUiType' perhaps?
-
-
-
reviewboard/static/rb/js/views/abstractReviewableView.es6.js (Diff revision 12) This isn't important for this review request but in my AJAX function review request, there was a check for whether a comment should be rendered or not here. Since that will likely be useful for your JSON review UI if you rebase on top of this review request, you should remember to add that here in your JSON code.
-
reviewboard/static/rb/js/views/markdownReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) The docstring should be modified to include the 'renderType' parameter since you are using it
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
reviewboard/static/rb/js/views/textBasedReviewableView.es6.js (Diff revision 12) This test code should be removed
-
-