Closed Bug 1326100 Opened 3 years ago Closed 3 years ago

Allow inline comments in ESLint config


(DevTools :: General, defect, P3)

49 Branch


(firefox55 fixed)

Firefox 55
Tracking Status
firefox55 --- fixed


(Reporter: jryans, Assigned: jryans)




(2 files)

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...
Tom, Patrick, any opinions?
Flags: needinfo?(ttromey)
Flags: needinfo?(pbrosset)
(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.
Flags: needinfo?(ttromey)
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?
Flags: needinfo?(pbrosset)
Another form that this rule disallows:

const POPUP_HIDDEN_DELAY = 100; // ms

Currently, the rule would have you rewrite this as:

// ms

which seems sub-par, since the unit is now further from the number it describes.
Assignee: nobody → jryans
Comment on attachment 8849287 [details]
Bug 1326100 - Allow inline comments in DevTools.

Thank you.
Attachment #8849287 - Flags: review?(ttromey) → review+
Comment on attachment 8849288 [details]
Bug 1326100 - Restore notable inline comments in DevTools.

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.
Attachment #8849288 - Flags: review?(ttromey) → review+
Pushed by
Allow inline comments in DevTools. r=tromey
Restore notable inline comments in DevTools. r=tromey
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.