Closed Bug 1488607 Opened Last year Closed Last year

Adapt to ESLint rule changes in bug 1488445


(Thunderbird :: General, task)

Not set


(Not tracked)

Thunderbird 64.0


(Reporter: darktrojan, Assigned: darktrojan)



(1 file)

M-C have updated the recommended rules. The only one affecting us is no-native-reassign replaced with no-global-assign.
I've also cleaned up a redundant rule in mailnews.
Attachment #9006414 - Flags: review?(jorgk)
Comment on attachment 9006414 [details] [diff] [review]

Thanks. I had noticed the redundant rule in mailnews/, but I was tired after a 300+KB patch review. Thanks for cleaning up.
Attachment #9006414 - Flags: review?(jorgk) → review+
Pushed by
Adapt to ESLint rule changes in bug 1488445. r=jorgk
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 64.0
(In reply to Geoff Lankow (:darktrojan) from comment #1)
> Created attachment 9006414 [details] [diff] [review]
> 1488607-eslint-1.diff
> I've also cleaned up a redundant rule in mailnews.

Did you verify this? The following no longer raises an error locally if a dangling comma is omitted:
/mozilla-central/comm$ ../mach lint mailnews/extensions/newsblog/

There is an override in mozilla-central for files /extensions/** and comma-dangle off.
> $ ../mach lint mailnews/extensions/newsblog/
> /home/geoff/mozilla/mozilla-central/comm/mailnews/extensions/newsblog/js/newsblog.js
>  32:77  error  Missing trailing comma.  comma-dangle (eslint)
> ✖ 1 problem (1 error, 0 warnings)

Is your mozilla-central up-to-date?
m-c tip is c4c125ee2556, c-c tip is 5b9052faa6b4.  Removing the comma in the same place as above raises no error.
Please revert the change and restore the comma-dangle rule in my original patch. If you reread comment 4 you will see why, here let me show you - You may also want to check your m-c up-to-dateness.

I would suggest better etiquette is more NIing and less gunslinging..
Flags: needinfo?(geoff)
Alta88, the link you gave doesn't work. It's preferable to point to M-C files via "mozilla-central" and skip the "mozilla" bit.

I don't really want to get involved in what looks like a bit of friction ;-) - but to follow up, Geoff did this try run:
Removing the comma here
does trigger the rule.

I cannot confirm your comment #4 locally: ../mach lint mailnews/extensions/newsblog/ gives me
  32:77  error  Missing trailing comma.  comma-dangle (eslint)

So may I consider this issue as resolved or am I missing something?
Flags: needinfo?(geoff) overrides /extensions absent a specific rule lower in the comm subdir. So it does not work for me, with latest tips on m-c and c-c, no. Do you not locally have that m-c/eslintrc.js as in dxr?
I'm on tip most of the time since I'm doing bustage fixes. Yes, the DXR link reflects what I have locally. It's working for me locally, I've just verified that again. Does M-C's .eslintrc.js get applied? C-C has it's own. Sorry I can't be of further help.
Changing that rule in m-c/.eslintrc.js locally has no effect, I have no idea what's going on or why if wfy. Maybe you have a global eslint setup, in /home or elsewhere. I used to have one and removed it. Maybe I need fresh clones. In any case, thanks for trying to help, rather than trampling my patch, making it not wfm, and walking away.
Type: enhancement → task
You need to log in before you can comment on or make changes to this bug.