Summary

JS resources now use functions for defaults

Review Request #7381 — Created June 4, 2015 and submitted June 8, 2015, 9:11 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.0.x

Bugs

Depends On

Blocks

7393

Commit

11da7d2...

Reviewers

Groups

reviewboard

People

Description

Previously, the RB.BaseResource model was using an object literal for

its defaults, but its default value for extra data was an empty object.

This could possibly result in shared state if a resource did not return

an extra_data JSON field, so the defaults property has been changed

into a function to avoid this.
All child resources of RB.BaseResource have been modified to use a

function instead of an attribute for defaults and to correctly invoke

the parent class's definition of defaults.

Testing Done

Ran JS tests.

Issues

Description	From	Last Updated
The line is now too long. I suspect there are others, but I can't easily check here (we really need …	chipx86	June 8, 2015, 10:19 a.m.
Not sure we need to bother with the .call(this), since the context doesn't actually matter in any of these (nor …	chipx86	June 8, 2015, 10:22 a.m.
This is over-indented.	chipx86	June 8, 2015, 10:23 a.m.

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/resources/models/baseResourceModel.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/resources/models/baseResourceModel.js

```
Testing done?
```

Description:

		Previously, the `RB.BaseResource` model was using an object literal for
		its defaults, but its default value for extra data was an empty object.
		This could possibly result in shared state if a resource did not return
		an `extra_data` JSON field, so the `defaults` property has been changed
		into a function to avoid this.
	+
	+	All child resources of `RB.BaseResource` have been modified to use a
	+	function instead of an attribute for `defaults` and to correctly invoke
	+	the parent class's definition of `defaults`.

Testing Done:

Ran JS tests.

Commit:

6e28dde89e26371366f4b4767282f378309636a7

4c43fa7de12469bd23b6b7e913fadbfb3b7ab802

Diff:

Revision 2 (+309 -273)

Show changes

	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
	reviewboard/static/rb/js/resources/models/diffModel.js
	reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
	reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
	10 more

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js

Summary:: BaseResource now uses function for defaults
JS resources now use functions for defaults

Ship it!

```
Ship It!
```

reviewboard/static/rb/js/resources/models/baseCommentModel.js (Diff revision 2)

The issue has been resolved. Show all issues

The line is now too long.

I suspect there are others, but I can't easily check here (we really need that feature).

reviewboard/static/rb/js/resources/models/baseCommentModel.js (Diff revision 2)

The issue has been resolved. Show all issues

Not sure we need to bother with the .call(this), since the context doesn't actually matter in any of these (nor should they, as the defaults functions shouldn't assume any information on the instance). (Doesn't hurt, though, and I can be convinced otherwise, but worth bringing up.)

reviewboard/static/rb/js/resources/models/defaultReviewerModel.js (Diff revision 2)
The issue has been resolved. Show all issues
```
This is over-indented.
```

Change Summary:

Addressed issues.

Commit:

4c43fa7de12469bd23b6b7e913fadbfb3b7ab802

29bc27b8d220abece46198adb562ce8651dcb084

Diff:

Revision 3 (+311 -273)

Show changes

	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
	reviewboard/static/rb/js/resources/models/diffModel.js
	reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
	reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
	10 more

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js

Change Summary:

Fix some typos

Commit:

29bc27b8d220abece46198adb562ce8651dcb084

59ddf1d2f21a5e36f05880f53b29ac2bc1200fb0

Diff:

Revision 4 (+311 -273)

Show changes

	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
	reviewboard/static/rb/js/resources/models/diffModel.js
	reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
	reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
	10 more

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js

Change Summary:

Fix typo

Commit:

59ddf1d2f21a5e36f05880f53b29ac2bc1200fb0

11da7d241dd0dd4296f11312a47bc06302e44e41

Diff:

Revision 5 (+311 -273)

Show changes

	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
	reviewboard/static/rb/js/resources/models/diffModel.js
	reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
	reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
	10 more

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/resources/models/reviewGroupModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentModel.js
    reviewboard/static/rb/js/resources/models/reviewRequestModel.js
    reviewboard/static/rb/js/resources/models/diffCommentModel.js
    reviewboard/static/rb/js/resources/models/diffModel.js
    reviewboard/static/rb/js/resources/models/fileDiffModel.js
    reviewboard/static/rb/js/resources/models/reviewReplyModel.js
    reviewboard/static/rb/js/resources/models/screenshotCommentModel.js
    reviewboard/static/rb/js/resources/models/baseResourceModel.js
    reviewboard/static/rb/js/resources/models/baseCommentModel.js
    reviewboard/static/rb/js/resources/models/draftReviewRequestModel.js
    reviewboard/static/rb/js/resources/models/screenshotModel.js
    reviewboard/static/rb/js/resources/models/fileAttachmentCommentModel.js
    reviewboard/static/rb/js/resources/models/defaultReviewerModel.js
    reviewboard/static/rb/js/resources/models/baseCommentReplyModel.js
    reviewboard/static/rb/js/resources/models/validateDiffModel.js
    reviewboard/static/rb/js/resources/models/repositoryModel.js
    reviewboard/static/rb/js/resources/models/reviewModel.js

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-2.0.x (e73dc19)