Summary

Don't include ) or ] in bug link

Review Request #10594 — Created June 15, 2019 and submitted July 29, 2019, 9:59 a.m.

Information

Owner

erijo

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

Makes references like "(see issue #abc)" work as expected.

Also modify existing linkifyText test lines to be <= 79 chars.

Testing Done

Unit test.

Commits

Summary	ID
Don't include ) or ] in bug link Makes references like "(see issue #abc)" work as expected. Also modify existing linkifyText test lines to be <= 79 chars.	444b21c8da6d2c23205da3fbc4c7c6694c026d11

Issues

Description	From	Last Updated
Can you make sure this fits in <= 79 chars? I know some of the others have this issue as …	chipx86	July 15, 2019, 9:17 a.m.
Since this is an ES6 file we should use const instead of var. Let's also add a blank line between …	david	July 19, 2019, 1:45 a.m.
Col: 53 Script URL.	reviewbot	July 11, 2019, 1:31 p.m.
Col: 55 Script URL.	reviewbot	July 11, 2019, 1:31 p.m.
Can you add a blank line after this?	david	July 19, 2019, 1:45 a.m.
const, blank line.	david	July 19, 2019, 1:45 a.m.
Col: 53 Script URL.	reviewbot	July 19, 2019, 1:45 a.m.
Col: 55 Script URL.	reviewbot	July 19, 2019, 1:45 a.m.

flake8 passed.

JSHint passed.

```
Any comments on this?
```

Thanks for the ping on this. We've been pretty busy with a major update of everything for Django 1.11 and eventually Python 3 (btw, only planned to be officially supported under RB5, RB4 with a custom contract). So we haven't been looking much at contributions. I'll get this in for 3.0, though, after the below fix.

reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Can you make sure this fits in <= 79 chars? I know some of the others have this issue as well, but you could update this one to save the linkified text in a variable and then just compare the variable.

erijo July 11, 2019, 1:30 p.m.

I applied a slightly different solution to make all lines be <= 79 chars. If you don't like it I can revert it.

Change Summary:

Make all linkifyText test lines be <= 79 chars.

Description:

		Makes references like "(see issue #abc)" work as expected.
	+
	+	Also modify existing linkifyText test lines to be <= 79 chars.

Commits:

	Summary	ID
	Don't include ) or ] in bug link Makes references like "(see issue #abc)" work as expected.	6dcee1b20abff90466cd4ee20a62f33a57f23e89
	Don't include ) or ] in bug link Makes references like "(see issue #abc)" work as expected. Also modify existing linkifyText test lines to be <= 79 chars.	19bf0b5ff5ca61eb1940ec088a35b9b24b22535e

Diff:

Revision 2 (+86 -60)

Show changes

	reviewboard/static/rb/js/utils/linkifyUtils.es6.js
	reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 53
 Script URL.
```
reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Col: 55
 Script URL.
```

reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 2)

The issue has been resolved. Show all issues

Since this is an ES6 file we should use const instead of var. Let's also add a blank line between this line and the next for readability.

reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 2)
The issue has been resolved. Show all issues
```
Can you add a blank line after this?
```
reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 2)
The issue has been resolved. Show all issues
```
const, blank line.
```

Change Summary:

var -> const and new lines

Commits:

	Summary	ID
	Don't include ) or ] in bug link Makes references like "(see issue #abc)" work as expected. Also modify existing linkifyText test lines to be <= 79 chars.	19bf0b5ff5ca61eb1940ec088a35b9b24b22535e
	Don't include ) or ] in bug link Makes references like "(see issue #abc)" work as expected. Also modify existing linkifyText test lines to be <= 79 chars.	444b21c8da6d2c23205da3fbc4c7c6694c026d11

Diff:

Revision 3 (+92 -60)

Show changes

	reviewboard/static/rb/js/utils/linkifyUtils.es6.js
	reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js

Checks run (1 failed, 1 succeeded)

flake8 passed.

JSHint failed.

JSHint

reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 53
 Script URL.
```
reviewboard/static/rb/js/utils/tests/linkifyUtilsTests.es6.js (Diff revision 3)
The issue has been dropped. Show all issues
```
Col: 55
 Script URL.
```

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to release-4.0.x (350775b)