Summary

Convert js/resources/models to ES6.

Review Request #8108 — Created April 8, 2016 and submitted May 5, 2016, 8:07 p.m.

Information

Owner

david

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Commit

851ad83...

Reviewers

Groups

reviewboard

People

Description

This is a relatively large but straightforward conversion.

Testing Done

Ran unit tests.

Issues

Description	From	Last Updated
You can make this const and do: const url = /* ... */; return this.isNew() ? url : `${url}${this.id}/`;	brennie	May 5, 2016, 5:35 p.m.
Do we prefer spread/rest over arguments? e.g., { destroyIfEmpty(...args) { if (!this.get('text')) { this.destroy.apply(this, args); } } }	brennie	May 5, 2016, 5:40 p.m.
Add a period.	brennie	May 5, 2016, 5:36 p.m.
"Serialize and return"	brennie	May 5, 2016, 5:37 p.m.
What about: { deserializers: { date(date) { return date ? new Date(date) : ''; }, summary(message) { return message.split('\n', 1)[0]; …	brennie	May 5, 2016, 5:37 p.m.
Same here.	brennie	May 5, 2016, 5:38 p.m.
parseResourceData(rsp)	brennie	May 5, 2016, 5:38 p.m.
Same comment as the other one about making this const.	brennie	May 5, 2016, 5:36 p.m.
Same as above.	brennie	May 5, 2016, 5:36 p.m.
Same as above.	brennie	May 5, 2016, 5:36 p.m.
Same comment re: function literals.	brennie	May 5, 2016, 5:38 p.m.
Same comment about const-ness.	brennie	May 5, 2016, 5:36 p.m.
parseResourceData(rsp)	brennie	May 5, 2016, 5:39 p.m.
validate(attrs)	brennie	May 5, 2016, 5:39 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/screenshotCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.es6.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/utils/serializers.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/draftFileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffModel.es6.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/draftResourceModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.es6.js



Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/screenshotCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.es6.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/utils/serializers.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/draftFileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffModel.es6.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/draftResourceModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.es6.js

Ship it!

```
Some stylistic nitpicks. Nothing major.
```

reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

You can make this const and do:

const url = /* ... */;

return this.isNew() ? url : `${url}${this.id}/`;

reviewboard/static/rb/js/resources/models/baseCommentReplyModel.es6.js (Diff revision 1)

The issue has been dropped. Show all issues

Do we prefer spread/rest over arguments? e.g.,

{
    destroyIfEmpty(...args) {
        if (!this.get('text')) {
            this.destroy.apply(this, args);
        }
    }
}

david May 5, 2016, 5:40 p.m.

I don't feel strongly about it. If we decide to go that route I can make another pass.

reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Add a period.
```
reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
"Serialize and return"
```

reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js (Diff revision 1)

The issue has been dropped. Show all issues

What about:

{
deserializers: {
    date(date) {
        return date ? new Date(date) : '';
    },
    summary(message) {
        return message.split('\n', 1)[0];
    }
}
}

david May 5, 2016, 5:40 p.m.

That seems longer for no benefit. The new function sugar is nice for things that look like classes (and may eventually become classes), but I don't think it's always worth it.

reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js (Diff revision 1)
The issue has been dropped. Show all issues
```
Same here.
```
reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
parseResourceData(rsp)
```
reviewboard/static/rb/js/resources/models/repositoryModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comment as the other one about making this const.
```
reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Same as above.
```
reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Same as above.
```

reviewboard/static/rb/js/resources/models/reviewModel.es6.js (Diff revision 1)

The issue has been dropped. Show all issues

Same comment re: function literals.

reviewboard/static/rb/js/resources/models/reviewReplyModel.es6.js (Diff revision 1)

You can do ready() { ... } I believe. Not sure if this will be correct, though.

david May 5, 2016, 5:40 p.m.
```
Yeah, I want the lexical this binding.
```

reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comment about const-ness.
```
reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
parseResourceData(rsp)
```
reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
validate(attrs)
```

Commit:

12e0fd7829c7f63ff61ecded259171b4b3719101

851ad83da035a8071e65856552ccbe1046c3c81e

Diff:

Revision 2 (+2222 -1557)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/resources/models/apiTokenModel.js
	reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
	reviewboard/static/rb/js/resources/models/baseCommentModel.js
	reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
	reviewboard/static/rb/js/resources/models/baseResourceModel.js
	reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
	reviewboard/static/rb/js/resources/models/diffCommentModel.js
	35 more

Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/screenshotCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.es6.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/utils/serializers.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/draftFileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffModel.es6.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.js
    reviewboard/static/rb/js/resources/models/draftResourceModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.es6.js



Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/resources/models/screenshotCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.es6.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/repositoryCommitModel.js
    reviewboard/static/rb/js/resources/models/repositoryBranchModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.es6.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.es6.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.es6.js
    reviewboard/static/rb/js/resources/models/diffCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/utils/serializers.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/draftResourceChildModelMixin.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/draftFileAttachmentModel.es6.js
    reviewboard/static/rb/js/resources/models/diffModel.es6.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/reviewGroupModel.es6.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewModel.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.js
    reviewboard/static/rb/js/resources/models/draftResourceModelMixin.es6.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/generalCommentReplyModel.es6.js
    reviewboard/static/rb/js/resources/models/apiTokenModel.es6.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/draftReviewModel.es6.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.es6.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.es6.js

Status:: Completed
Change Summary:: Pushed to release-3.0.x (41e48b8)