Don't include ) or ] in bug link

Review Request #10594 — Created June 15, 2019 and submitted

erijo
Review Board
release-3.0.x
reviewboard
Makes references like "(see issue #abc)" work as expected.

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

Unit test.

Summary
Don't include ) or ] in bug link
Description From Last Updated

Can you make sure this fits in <= 79 chars? I know some of the others have this issue as ...

chipx86chipx86

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

daviddavid

Col: 53 Script URL.

reviewbotreviewbot

Col: 55 Script URL.

reviewbotreviewbot

Can you add a blank line after this?

daviddavid

const, blank line.

daviddavid

Col: 53 Script URL.

reviewbotreviewbot

Col: 55 Script URL.

reviewbotreviewbot
erijo
  1. Any comments on this?

  2. 
      
chipx86
  1. 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.

  2. 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.

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

  3. 
      
erijo
Review request changed

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
-
Don't include ) or ] in bug link
+
Don't include ) or ] in bug link

Diff:

Revision 2 (+86 -60)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

david
  1. 
      
  2. 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.

  3. Can you add a blank line after this?

  4. 
      
erijo
Review request changed

Change Summary:

var -> const and new lines

Commits:

Summary
-
Don't include ) or ] in bug link
+
Don't include ) or ] in bug link

Diff:

Revision 3 (+92 -60)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 passed.
JSHint failed.

JSHint

david
  1. Ship It!
  2. 
      
erijo
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (350775b)
Loading...