Summary

Fix up the formatting of the repository form JS

Review Request #7761 — Created Nov. 9, 2015 and submitted Nov. 12, 2015, 8:47 a.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.5.x

Bugs

Depends On

Blocks

7757

Reviewers

Groups

reviewboard

People

Description

This patch brings repositoryform.js into line with our style
guidelines.

Testing Done

Verified that the repository form still worked.
Ran JSHint.

Issues

Description	From	Last Updated
This would be better written if we flip it: $('<p class="help" />') .text(text) .appendTo($('#row-' + field));	david	Nov. 9, 2015, 2:17 p.m.
Please undo the indentation change here (we want the ? and : to align within the parens)	david	Nov. 9, 2015, 2:17 p.m.
This can be: $hostingAccountPassRow.setVisible(hostingInfo.needs_authorization);	david	Nov. 9, 2015, 2:18 p.m.
This is awkward. Can we store $hostingAccount[0].selectedIndex] into a variable first? We should probably also call this $selectedOption since it's …	david	Nov. 9, 2015, 2:19 p.m.
setVisible()	david	Nov. 9, 2015, 2:20 p.m.
setVisible()	david	Nov. 9, 2015, 2:21 p.m.
Let's wrap $(this) once.	david	Nov. 9, 2015, 6:53 p.m.
Please undo this indentation change.	david	Nov. 9, 2015, 6:53 p.m.
This can use setVisible.	david	Nov. 9, 2015, 6:54 p.m.
setVisible	david	Nov. 9, 2015, 6:56 p.m.
Probably better as $hostingType.val()	david	Nov. 9, 2015, 6:56 p.m.
Probably better as $hostingType.val()	david	Nov. 9, 2015, 6:59 p.m.
Probably better as $bugTrackerType.val()	david	Nov. 9, 2015, 6:59 p.m.

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js

Diff:

Revision 2 (+178 -175)

Show changes

reviewboard/static/rb/js/repositoryform.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js

reviewboard/static/rb/js/repositoryform.js (Diff revision 2)

The issue has been resolved. Show all issues

This would be better written if we flip it:

$('<p class="help" />')
    .text(text)
    .appendTo($('#row-' + field));

reviewboard/static/rb/js/repositoryform.js (Diff revision 2)

The issue has been resolved. Show all issues

Please undo the indentation change here (we want the ? and : to align within the parens)

reviewboard/static/rb/js/repositoryform.js (Diff revision 2)

The issue has been resolved. Show all issues

This can be:

$hostingAccountPassRow.setVisible(hostingInfo.needs_authorization);

reviewboard/static/rb/js/repositoryform.js (Diff revision 2)

The issue has been resolved. Show all issues

This is awkward. Can we store $hostingAccount[0].selectedIndex] into a variable first?

We should probably also call this $selectedOption since it's a jQuery

reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
The issue has been resolved. Show all issues
```
setVisible()
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 2)
The issue has been resolved. Show all issues
```
setVisible()
```

Diff:

Revision 3 (+181 -185)

Show changes

reviewboard/static/rb/js/repositoryform.js

Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js



Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js

Found some more stuff. My word this code is old and crusty.

reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Let's wrap $(this) once.
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Please undo this indentation change.
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
This can use setVisible.
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
setVisible
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Probably better as $hostingType.val()
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Probably better as $hostingType.val()
```
reviewboard/static/rb/js/repositoryform.js (Diff revision 3)
The issue has been resolved. Show all issues
```
Probably better as $bugTrackerType.val()
```

Change Summary:

Address David's issues & use $.fn.prop() over $.fn.attr()

Diff:

Revision 4 (+194 -193)

Show changes

reviewboard/static/rb/js/repositoryform.js

Tool: Pyflakes
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js



Tool: PEP8 Style Checker
Ignored Files:
    reviewboard/static/rb/js/repositoryform.js

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (45bd7b0)