Front-end for general comments
Review Request #6506 — Created Oct. 25, 2014 and submitted
Information | |
---|---|
nicolexin | |
Review Board | |
master | |
33ec3a2... | |
Reviewers | |
reviewboard, students | |
Reviewboard currently support 3 types of comments: Diff commment, File attachment comments, Screenshot comments.
This is a new type of comments: General comments.A general comment on a review request is used when a comment is not tied to specific lines of code or a special file attachment, and an issue is opened. Examples include suggestions for testing or pointing out errors in the change description.
The general comment backend model is built. (review request 6375)
The general comment web-api also done. (review request 6431)
This review request is for implemention of general comments at front-end.
- Now the general comment can be displayed as a review box on the page
- Issue bar is working (e.g. fix/drop/reopen the issue) and the issue summary seems good.
- A email is sent when a general comment is created and replied.
How to create a general comment:
1. On a review request page, click ‘Add Comment’ to the right of ‘Review’ button.
2. A green box will pop up. Write your general comment and tick the checkboxes if needed.
3. Click ‘OK’ to save the comment, then the green box will disappear and there will be a review draft banner on the top of the page.
4. If you wish to have multiple general comments in the same review, repeat step 1-3.
5. Once finished, click 'Publish' on the review draft banner to publish the review. (or you can discard or edit the review just like publishing other comments)
All js-tests passed.
Tests including:
1. reviewReplyEditor with general comments
2. General comment model
3. reviewReply with general comments
4. reviewDialogView with general comments
Description | From | Last Updated |
---|---|---|
Indentation issue? Tabs? |
|
|
No need for this, since we're not extending anything. Just assign directly without _.defaults(). |
|
|
Missing trailing comma. |
|
|
Reviewables are just there to provide a way to review files. We won't need to provide this for general comments, … |
|
|
By passing an ID, you're telling the comment dialog that this comment already exists on the server, which isn't true. … |
|
|
It should probably return null, instead of false. When comparing in JavaScript, you want to use === instead of ==. … |
|
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
Col: 13 E265 block comment should start with '# ' |
![]() |
|
It displays a comment dialog, not a review dialog. |
|
|
This is the default, so no need to specify it. |
|
|
This needs documentation above it. |
|
|
This will happen automatically. No need to specify it. |
|
|
This is the default. No need to specify it. |
|
|
These are also all defaults. No need to override. |
|
|
The trailing comma must be removed. In JavaScript, this is technically a syntax error, which will break IE and some … |
|
|
This isn't the right place for this. Instead, the code that sets up the dialog for creating new general comments … |
|
|
Doesn't look like this got fixed? Also, missing space before the {. |
|
|
We should probably load these before other comments, so that they'll appear at the top of the review. |
|
|
Should be aligned like the other ones. |
|
|
For all the HTML you're introducing, make sure you're using 1-space indentation, and that it's aligned with respect to the … |
|
|
Missing a var statement. |
|
|
I'd rather not introduce a new event for this (which we do below). Instead, let's do what the other call … |
|
|
This was marked as fixed in two of my prior reviews, but isn't fixed. This function doesn't return a boolean, … |
|
|
Blank line between these. This should also be generalComment, or probably comment. |
|
|
We only use one var statement. So above, do: var comment = ..., dlg; |
|
|
This isn't needed anymore. |
|
-
We just made some changes to some files that may result in some merge difficulties.
When you get a chance, I'd like to see a screenshot of an e-mail with general comments.
-
reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js (Diff revision 1) Indentation issue? Tabs?
-
reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js (Diff revision 1) No need for this, since we're not extending anything. Just assign directly without
_.defaults()
. -
reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js (Diff revision 1) Missing trailing comma.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 2 (+326 -4) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/resources/models/reviewModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/resources/models/reviewModel.js
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 3 (+355 -5) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/pages/views/reviewablePageView.js
-
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 3) By passing an ID, you're telling the comment dialog that this comment already exists on the server, which isn't true. You shouldn't pass an ID.
Also note how the data you're passing doesn't match the function signature for createGeneralComment.
-
-
reviewboard/static/rb/js/models/generalReviewableModel.js (Diff revision 3) Reviewables are just there to provide a way to review files. We won't need to provide this for general comments, since "General" isn't a type of reviewable thing. General comments are independent of any files.
Likewise, you won't need GeneralCommentBlock.
-
reviewboard/static/rb/js/views/reviewDialogView.js (Diff revision 3) It should probably return null, instead of false.
When comparing in JavaScript, you want to use
===
instead of==
. Seems weird, but that's how it works.===
is a string comparison, whereas==
attempts to cast things.Also, blank line between this conditional and the next return statement.
Description: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 4 (+407 -5) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/views/generalReviewableView.js reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/notifications/review_email.html reviewboard/static/rb/js/pages/views/reviewablePageView.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/templates/reviews/boxes/review.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/views/generalReviewableView.js reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/static/rb/js/models/generalCommentBlockModel.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/notifications/review_email.html reviewboard/static/rb/js/pages/views/reviewablePageView.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/static/rb/js/models/generalReviewableModel.js reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/templates/reviews/boxes/review.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js
-
-
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 5 (+360 -6) |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js
Summary: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Testing Done: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 6 (+360 -6) |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Added Files: |

-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js
-
One more piece of feedback, unrelated to code. Each review request's summary, description, and testing done should be fully self-descriptive, allowing a reviewer or someone down the road to read through it and know exactly what you're doing, why, and how, without having to dive into the code. We'll also be using these for the commit message. This also massively helps when we later are looking for the source of regressions, or are drafting release notes.
You can browse through any of my commits in the repository for examples of how these should look.
-
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 6) It displays a comment dialog, not a review dialog.
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 6) This is the default, so no need to specify it.
-
reviewboard/static/rb/js/resources/models/generalCommentModel.js (Diff revision 6) This needs documentation above it.
-
reviewboard/static/rb/js/resources/models/generalCommentModel.js (Diff revision 6) This will happen automatically. No need to specify it.
-
reviewboard/static/rb/js/resources/models/generalCommentModel.js (Diff revision 6) This is the default. No need to specify it.
-
reviewboard/static/rb/js/resources/models/generalCommentModel.js (Diff revision 6) These are also all defaults. No need to override.
-
reviewboard/static/rb/js/resources/models/reviewModel.js (Diff revision 6) The trailing comma must be removed. In JavaScript, this is technically a syntax error, which will break IE and some other browsers.
-
reviewboard/static/rb/js/views/commentDialogView.js (Diff revision 6) This isn't the right place for this. Instead, the code that sets up the dialog for creating new general comments on the review request page should call this when the comment has been saved.
-
reviewboard/static/rb/js/views/reviewDialogView.js (Diff revision 6) Doesn't look like this got fixed?
Also, missing space before the
{
. -
reviewboard/static/rb/js/views/reviewDialogView.js (Diff revision 6) We should probably load these before other comments, so that they'll appear at the top of the review.
-
reviewboard/templates/notifications/review_email.html (Diff revision 6) Should be aligned like the other ones.
-
reviewboard/templates/reviews/boxes/review.html (Diff revision 6) For all the HTML you're introducing, make sure you're using 1-space indentation, and that it's aligned with respect to the other HTML.
Also make sure that the nesting looks correct. For instance, here, it looks like the
<div>
is nested inside the<a>
, but it's not. The</div>
also isn't aligning with the<div>
.This will all be a bit easier to see when you've made sure they're aligned with other HTML and using the proper indentation size.
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 7 (+344 -11) |
-
File Captions:
Screen Shot 2014-11-16 at 7.52.55 PM.png: - With one reply + Email with one reply Screen Shot 2014-11-16 at 7.53.21 PM.png: - With multiple replies + Email with multiple replies
Added Files: |
---|

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js
-
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 7) Missing a
var
statement. -
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 7) I'd rather not introduce a new event for this (which we do below). Instead, let's do what the other call sites do, which is create the comment and listen to that comment's 'saved' event instead.
-
reviewboard/static/rb/js/views/reviewDialogView.js (Diff revision 7) This was marked as fixed in two of my prior reviews, but isn't fixed. This function doesn't return a boolean, but rather must return a string or null.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 8 (+346 -11) |

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/views/commentDialogView.js reviewboard/static/rb/js/pages/views/reviewablePageView.js
-
Unless I missed it, it might be useful to upload a view (or just describe in words in the description) of how to actually create a GeneralComment. For example, how did you get the green display box to submit the issue/general comment?
-
Last 3 changes! Then it's going in :)
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 8) Blank line between these.
This should also be
generalComment
, or probablycomment
. -
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 8) We only use one
var
statement. So above, do:var comment = ..., dlg;
-
Description: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Commit: |
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Diff: |
Revision 9 (+347 -11) |

-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/models/commentIssueManagerModel.js reviewboard/static/rb/js/views/reviewDialogView.js reviewboard/templates/notifications/reply_email.html reviewboard/static/rb/js/resources/models/reviewReplyModel.js reviewboard/templates/notifications/reply_email.txt reviewboard/static/rb/js/resources/models/generalCommentModel.js reviewboard/static/rb/js/resources/models/tests/reviewReplyModelTests.js reviewboard/templates/reviews/boxes/review.html reviewboard/static/rb/js/views/tests/reviewDialogViewTests.js reviewboard/templates/notifications/review_email.html reviewboard/templates/reviews/review_request_actions_primary.html reviewboard/static/rb/js/resources/models/tests/generalCommentModelTests.js reviewboard/static/rb/js/resources/models/generalCommentReplyModel.js reviewboard/static/rb/js/resources/models/reviewModel.js reviewboard/static/rb/js/models/reviewReplyEditorModel.js reviewboard/templates/notifications/review_email.txt reviewboard/static/rb/js/models/tests/reviewReplyEditorModelTests.js reviewboard/static/rb/js/pages/views/reviewablePageView.js