Summary

[WIP] Mobile diff viewer project

Review Request #9266 — Created Oct. 13, 2017 and discarded Nov. 30, 2017, 5:31 p.m.

Information

Owner

tbrockma

Repository

Review Board

Branch

master

Bugs

Depends On

Reviewers

Groups

reviewboard, students

People

Description

wip don't look here

Testing Done

Issues

Description	From	Last Updated
Col: 10 Missing semicolon.	reviewbot	Oct. 13, 2017, 8:07 p.m.
Col: 10 Missing semicolon.	reviewbot	Oct. 13, 2017, 8:50 p.m.
Col: 10 Missing semicolon.	reviewbot	Oct. 13, 2017, 8:59 p.m.
Col: 32 Expected '!==' and instead saw '!='.	reviewbot	Oct. 13, 2017, 10:51 p.m.
Col: 13 Expected '{' and instead saw 'target'.	reviewbot	Oct. 13, 2017, 10:51 p.m.
W291 trailing whitespace	reviewbot	Oct. 22, 2017, 12:41 p.m.
E302 expected 2 blank lines, found 1	reviewbot	Nov. 5, 2017, 2:20 p.m.
E501 line too long (80 > 79 characters)	reviewbot	Nov. 5, 2017, 2:20 p.m.
F841 local variable 'cell_classes' is assigned to but never used	reviewbot	Nov. 5, 2017, 2:20 p.m.
F841 local variable 'cell_class_attr' is assigned to but never used	reviewbot	Nov. 5, 2017, 2:20 p.m.
It looks like you've got some stuff from your easyfix branch leaking into this.	david	Nov. 6, 2017, 12:42 a.m.

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js (Diff revision 1)
An issue was opened. Show all issues
```
Col: 10
 Missing semicolon.
```

Commit:

3cba3d541bc7d21b08cbd0a77dd95301b7a9ecf6

0ff2e92134e9a7f12bdcc23dadc3503df436e833

Diff:

Revision 2 (+351 -67)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
	reviewboard/templates/diffviewer/view_diff.html
	3 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js (Diff revision 2)
An issue was opened. Show all issues
```
Col: 10
 Missing semicolon.
```

Commit:

0ff2e92134e9a7f12bdcc23dadc3503df436e833

e84c668761c91de4397a1eb1ff712171e07d951e

Diff:

Revision 3 (+387 -75)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
	reviewboard/templates/diffviewer/view_diff.html
	3 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js (Diff revision 3)
An issue was opened. Show all issues
```
Col: 10
 Missing semicolon.
```

Commit:

e84c668761c91de4397a1eb1ff712171e07d951e

22e36a218ca297481c79e0563a6a1b41769daf57

Diff:

Revision 4 (+387 -75)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
	reviewboard/templates/diffviewer/view_diff.html
	3 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commit:

22e36a218ca297481c79e0563a6a1b41769daf57

d2eb64155c6101869e5afe77eba9052c8a44497b

Diff:

Revision 5 (+413 -85)

Show changes

	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/review-request.less
	reviewboard/static/rb/css/pages/reviews.less
	reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js
	reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
	4 more

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 32
 Expected '!==' and instead saw '!='.
```
reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.js (Diff revision 5)
An issue was opened. Show all issues
```
Col: 13
 Expected '{' and instead saw 'target'.
```

Description:: ~
beginning work on mobile stylings
~
wip don't look here
- beginning mobile diff viewer improvements
- before switching branches
- before switching to fix changes
- mobile diff summary
- working on expanding and contracting diff files
- allows clicking anywhere in the box, temporary modification of href color within diff list

Commit:

d2eb64155c6101869e5afe77eba9052c8a44497b

9a09566c814d4ab157ef444b0ee4ff5e4f490786

Diff:

Revision 6 (+522 -89)

Show changes

	reviewboard/diffviewer/renderers.py
	reviewboard/diffviewer/views.py
	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/diffviewer.less
	reviewboard/static/rb/css/pages/review-request.less
	9 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/views.py (Diff revision 6)
An issue was opened. Show all issues
```
W291 trailing whitespace
```

Commit:

9a09566c814d4ab157ef444b0ee4ff5e4f490786

fa7fd155e27fd17d821bae9866874a94d21a645d

Diff:

Revision 7 (+1091 -114)

Show changes

	reviewboard/diffviewer/views.py
	reviewboard/diffviewer/templatetags/difftags.py
	reviewboard/reviews/views.py
	reviewboard/reviews/models/review_request.py
	reviewboard/reviews/tests/test_review_request.py
	reviewboard/reviews/ui/base.py
	reviewboard/static/rb/css/pages/base.less
	reviewboard/static/rb/css/pages/diffviewer.less
	13 more

Checks run (1 failed, 1 succeeded)

flake8 failed.

JSHint passed.

flake8

reviewboard/diffviewer/templatetags/difftags.py (Diff revision 7)
An issue was opened. Show all issues
```
E302 expected 2 blank lines, found 1
```
reviewboard/diffviewer/templatetags/difftags.py (Diff revision 7)
An issue was opened. Show all issues
```
E501 line too long (80 > 79 characters)
```
reviewboard/diffviewer/templatetags/difftags.py (Diff revision 7)
An issue was opened. Show all issues
```
F841 local variable 'cell_classes' is assigned to but never used
```
reviewboard/diffviewer/templatetags/difftags.py (Diff revision 7)
An issue was opened. Show all issues
```
F841 local variable 'cell_class_attr' is assigned to but never used
```

Fix it!

It looks like you've got a bunch of different changes all kind of mixed together. Is it possible to extract things so that you have one review request per change?

tbrockma Nov. 7, 2017, 12:02 a.m.

I could do that in the future, but a lot of the stuff here isn't really anything I would want to put up for review until I've fixed things and written tests for each change.

It's a lot of hacky code just to get things working so I will definitely be putting time into cleaning everything up.

reviewboard/webapi/resources/review_request.py (Diff revision 7)

An issue was opened. Show all issues

It looks like you've got some stuff from your easyfix branch leaking into this.

tbrockma Nov. 7, 2017, 12:02 a.m.

Yeah, I'm not entirely certain why that happened, they both exist in their own separate branches but I guess at some point I merged incorrectly.

Status:: Discarded

~		beginning work on mobile stylings
	~	wip don't look here
-		beginning mobile diff viewer improvements
-		before switching branches
-		before switching to fix changes
-		mobile diff summary
-		working on expanding and contracting diff files
-		allows clicking anywhere in the box, temporary modification of href color within diff list