1326100 - Allow inline comments in ESLint config

Status: RESOLVED FIXED

(DevTools :: General, defect, P3)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Our current ESLint rules disallow inline comments (on the same line as code):

```
    // Disallow comments inline after code.
    "no-inline-comments": "error",
```

I think there several cases where these make sense to allow, including:

1. Noting the meaning of boolean args at call sites

```
  certOverrideService.rememberValidityOverride(host, port, cert, overrideBits,
                                               true /* temporary */);
```

2. Explaining a conditional branch

```
if (!Services.appinfo
    || Services.appinfo.processType == Services.appinfo.PROCESS_TYPE_CONTENT
    || Services.appinfo.ID === undefined /* XPCShell */
    || Services.appinfo.ID == B2G_ID
    || Services.appinfo.ID == GRAPHENE_ID
    || !AddonPathService) {
```

I am sure I can find more if needed...

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Tom, Patrick, any opinions?

Comment 2

[Tom Tromey :tromey]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #1)
> Tom, Patrick, any opinions?

I've also hit some cases where this would have been more clear.
I think it's ok when used judiciously, though of course there's no way
to explain that to eslint.  In the end though I think it's ok
to make this change.  Thanks for bringing this up.

Comment 3

[Patrick Brosset <:pbro>]

I have not met cases where this would have helped me personally, and I (personally, again) find examples 1. and 2. sort of hard to read.
But I don't have to deal with code much anymore these days, so I'll go with whatever other people say. Maybe something to bring up at the next meeting, or on the mailing list, and just land if nobody complains?

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Posted to dev-developer-tools:

https://groups.google.com/d/topic/mozilla.dev.developer-tools/VYZmFDgCnRw/discussion

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Another form that this rule disallows:

const POPUP_HIDDEN_DELAY = 100; // ms

Currently, the rule would have you rewrite this as:

// ms
const POPUP_HIDDEN_DELAY = 100;

which seems sub-par, since the unit is now further from the number it describes.

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1326100 - Allow inline comments in DevTools.

Review commit: https://reviewboard.mozilla.org/r/122104/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122104/

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: Bug 1326100 - Restore notable inline comments in DevTools.

Review commit: https://reviewboard.mozilla.org/r/122106/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/122106/

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=2c7090bb5f7a1583829011ef171544ea1c84c353

Comment 9

[Tom Tromey :tromey]

Comment on attachment 8849287 [details]
Bug 1326100 - Allow inline comments in DevTools.

https://reviewboard.mozilla.org/r/122104/#review124432

Thank you.

Comment 10

[Tom Tromey :tromey]

Comment on attachment 8849288 [details]
Bug 1326100 - Restore notable inline comments in DevTools.

https://reviewboard.mozilla.org/r/122106/#review124436

Thanks.  You really went the extra mile on this one!  It looks good and in particular I agree that all these comments are better inline.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0eba13bc9949
Allow inline comments in DevTools. r=tromey
https://hg.mozilla.org/integration/autoland/rev/b0e1eb0bd898
Restore notable inline comments in DevTools. r=tromey

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/0eba13bc9949
https://hg.mozilla.org/mozilla-central/rev/b0e1eb0bd898