Closed Bug 1403961 Opened 7 years ago Closed 7 years ago

Enable ESLint for ipc/

Categories

(Core :: IPC, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: standard8, Assigned: mccr8, Mentored)

References

Details

(Keywords: good-first-bug, Whiteboard: [lang=js])

Attachments

(2 files)

As part of rolling out ESLint across the tree, we should enable it for the ipc/ directory. I'm happy to mentor this bug. There's background on our eslint setups here: https://developer.mozilla.org/docs/ESLint Here's some approximate steps: - In .eslintignore, remove the `ipc/**` line. - Run eslint: ./mach eslint --fix ipc This should fix most/all of the issues. - Inspect the diff to make sure that the indentation of the lines surrounding the changes look ok. - For the remaining issues, you'll need to fix them by hand. I think most of these will be no-undef, there are hints on how to fix those here: https://developer.mozilla.org/en-US/docs/ESLint#no-undef (you can just run `./mach eslint ipc` to check you've fixed them). - Create commit of the changes and push it to mozreview: http://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview.html If you're not sure of how to fix something, please look at the rules documentation http://eslint.org/docs/rules/ or just ask here.
Blocks: 1357557
Assignee: nobody → continuation
This is the second part of the diff that seems to have broken MozReview: diff --git a/ipc/testshell/tests/test_ipcshell_child.js b/ipc/testshell/tests/test_ipcshell_child.js index d9c9fb6..2a0566b 100644 --- a/ipc/testshell/tests/test_ipcshell_child.js +++ b/ipc/testshell/tests/test_ipcshell_child.js @@ -3,7 +3,6 @@ var Ci = Components.interfaces; const runtime = Cc["@mozilla.org/xre/app-info;1"].getService(Ci.nsIXULRuntime); -if (typeof(run_test) == "undefined") { - run_test = function() { - do_check_eq(runtime.processType, Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT); - }^M} +function run_test() { + do_check_eq(runtime.processType, Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT); +} I guess it didn't like the ^M, whatever that is.
(In reply to Andrew McCreight (PTO-ish Oct 1 - 12) [:mccr8] from comment #4) > - }^M} > +function run_test() { > + do_check_eq(runtime.processType, Ci.nsIXULRuntime.PROCESS_TYPE_DEFAULT); > +} > > I guess it didn't like the ^M, whatever that is. The ^M is the representation of the carriage return - there were mixed line-endings in the child file.
Comment on attachment 8913843 [details] Bug 1403961, part 1 - Don't load test_ipcshell_child.js from test_ipcshell.js. https://reviewboard.mozilla.org/r/185230/#review190490 ::: ipc/testshell/tests/test_ipcshell.js (Diff revision 1) > do_check_eq(cmp, result); > do_test_finished(); > }); > }) > } > -load('test_ipcshell_child.js'); One alternative here would be to do: /* import-globals-from test_ipcshell_child.js */ ESLint will then detect that these are loaded from the child script.
(In reply to Mark Banner (:standard8) from comment #6) > One alternative here would be to do: The first fix I had was to add an annotation along those lines, but I decided I might as well take the opportunity to clean up a garbled test. (test_ipcshell_child.js used to do more, so maybe it used to make sense.)
Comment on attachment 8913843 [details] Bug 1403961, part 1 - Don't load test_ipcshell_child.js from test_ipcshell.js. https://reviewboard.mozilla.org/r/185230/#review190752
Attachment #8913843 - Flags: review?(wmccloskey) → review+
Attachment #8913844 - Flags: review?(wmccloskey) → review+
Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/6201576b6471 part 1 - Don't load test_ipcshell_child.js from test_ipcshell.js. r=billm https://hg.mozilla.org/integration/autoland/rev/cab06f0f811a part 2 - Fix and enable eslint for ipc/. r=billm
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: