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] 1488607-eslint-1.diff 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 firstname.lastname@example.org: https://hg.mozilla.org/comm-central/rev/4a2af5ca975b Adapt to ESLint rule changes in bug 1488445. r=jorgk
Status: ASSIGNED → RESOLVED
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 - https://searchfox.org/comm-central/rev/27b3d9c5fd2543f47c3943590c72fa560a5b6b9a/mozilla/.eslintrc.js#43. You may also want to check your m-c up-to-dateness. I would suggest better etiquette is more NIing and less gunslinging..
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: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=173433f969c978b91e11fdaf060005e7f9398bf7 Removing the comma here https://hg.mozilla.org/try-comm-central/rev/173433f969c978b91e11fdaf060005e7f9398bf7#l3.12 does trigger the rule. I cannot confirm your comment #4 locally: ../mach lint mailnews/extensions/newsblog/ gives me c:\mozilla-source\comm-central\comm\mailnews\extensions\newsblog\js\newsblog.js 32:77 error Missing trailing comma. comma-dangle (eslint) So may I consider this issue as resolved or am I missing something?
https://dxr.mozilla.org/mozilla-central/source/.eslintrc.js#43 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.
You need to log in before you can comment on or make changes to this bug.