Summary:

Update RB.DiffReviewableView to modern standards

Review Request #10194 — Created Oct. 4, 2018 and submitted Oct. 18, 2018, 9:39 p.m.

Information
Owner:	brennie
Repository:	Review Board
Branch:	release-4.0.x
Bugs:
Depends On:
Commit:	0d3a1df...
Reviewers
Groups:	reviewboard
People:

Description

This patch updates RB.DiffReviewableView and its unit tests to use

ES6 features and our documentation standards. Although the

RB.DiffReviewableView was written in an ES6 file, it was still using

var everywhere over let and const. It was also wrapped in a

superfluous IIFE.

Testing Done

Ran JS tests.

Issues

Description	From	Last Updated
Typo in description: Althought	david	Oct. 15, 2018, 1:20 p.m.
Indentation is funky here.	david	Oct. 15, 2018, 1:20 p.m.
This could be: $children.first().toggleClass('first'); $children.last().toggleClass('last');	david	Oct. 15, 2018, 1:20 p.m.
Extra blank line.	chipx86	Oct. 16, 2018, 2:30 p.m.
Can we keep one param per line here? Just nicer to read it that way.	chipx86	Oct. 16, 2018, 2:30 p.m.
Missing docs.	chipx86	Oct. 16, 2018, 2:33 p.m.
Missing Args.	chipx86	Oct. 16, 2018, 2:33 p.m.
"Handle"	chipx86	Oct. 16, 2018, 2:33 p.m.
This does not English good.	chipx86	Oct. 16, 2018, 2:33 p.m.
Missing a blank line here.	chipx86	Oct. 18, 2018, 9:10 p.m.

flake8 passed.

JSHint passed.

Show all issues
```
Typo in description: Althought
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 1)
Show all issues
```
Indentation is funky here.
```

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 1)

Show all issues

This could be:

$children.first().toggleClass('first');
$children.last().toggleClass('last');

Change Summary:

Addressed feedback.

Description:

		This patch updates `RB.DiffReviewableView` and its unit tests to use
~		ES6 features and our documentation standards. Althought the
	~	ES6 features and our documentation standards. Although the
		`RB.DiffReviewableView` was written in an ES6 file, it was still using
		`var` everywhere over `let` and `const`. It was also wrapped in a
		superfluous IIFE.

Commit:

-	6e6847a8480e74f85d2fce3903fa88312b0b41ff
+	e715ff66375607e37350594615b0c18177f7b258

Diff:

Revision 2 (+322 -288)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
	reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
Show all issues
```
Extra blank line.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
Show all issues
```
Can we keep one param per line here? Just nicer to read it that way.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
Show all issues
```
Missing docs.
```

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)

Show all issues

Missing Args.

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
Show all issues
```
"Handle"
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
Show all issues
```
This does not English good.
```

Change Summary:

Addressed Feedback.

Commit:

-	e715ff66375607e37350594615b0c18177f7b258
+	0d3a1df64d23709ab04a7460a53bf1a018646959

Diff:

Revision 3 (+329 -288)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
	reviewboard/static/rb/js/diffviewer/views/tests/diffReviewableViewTests.js

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 3)

Show all issues

Missing a blank line here.

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (af514f1)