Closed
Bug 1403961
Opened 7 years ago
Closed 7 years ago
Enable ESLint for ipc/
Categories
(Core :: IPC, enhancement)
Core
IPC
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.
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → continuation
Assignee | ||
Comment 1•7 years ago
|
||
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 4•7 years ago
|
||
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.
Reporter | ||
Comment 5•7 years ago
|
||
(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.
Reporter | ||
Comment 6•7 years ago
|
||
mozreview-review |
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.
Assignee | ||
Comment 7•7 years ago
|
||
(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 8•7 years ago
|
||
mozreview-review |
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+
Comment 9•7 years ago
|
||
mozreview-review |
Comment on attachment 8913844 [details]
Bug 1403961, part 2 - Fix and enable eslint for ipc/.
https://reviewboard.mozilla.org/r/185232/#review190754
Attachment #8913844 -
Flags: review?(wmccloskey) → review+
Comment 10•7 years ago
|
||
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
Comment 11•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6201576b6471
https://hg.mozilla.org/mozilla-central/rev/cab06f0f811a
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•