Fix camelcase warnings in webconsole.js

RESOLVED FIXED in Firefox 47

Status

()

Firefox
Developer Tools: Console
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: ajkerrigan, Assigned: ajkerrigan)

Tracking

Trunk
Firefox 47
Points:
---

Firefox Tracking Flags

(firefox47 fixed)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

2 years ago
+++ This bug was initially created as a clone of Bug #1243133 +++

After the brace-style formatting addressed in Bug 1243133, the next biggest chunk of ESLint violations comes from the camelcase rule (125 warnings).

The violations seem to come exclusively from method names. At first glance it looked like the naming was not in line with the MDN JavaScript practices (https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#JavaScript_practices), but webconsole.js has a XUL overlay and the method names do follow the advice set forth here: https://developer.mozilla.org/en-US/docs/JavaScript_Tips#Function_and_variable_naming

If the naming is good as is, perhaps the fix for this bug is as simple as a comment to disable ESLint camelcase checking:

/* eslint camelcase: 0 */

I'm happy to take this on or kick ideas around, just wanted to get some thoughts down first and avoid plowing ahead with a counterproductive fix.
(Assignee)

Updated

2 years ago
Blocks: 1232731
Flags: needinfo?(lclark)
Summary: Fix brace-style warnings in webconsole.js → Fix camelcase warnings in webconsole.js
Thanks for checking on this!

Just noting here for posterity:

We talked about this in #devtools. :gl and :tromey thought this was no longer necessary because it was put in place to help with debugging when stack traces weren't as nice. :ajkerrigan also found this documentation of the rule, which seems to be XUL related: https://developer.mozilla.org/en-US/docs/JavaScript_Tips#Function_and_variable_naming. We decided to give it a try removing the names.
Flags: needinfo?(lclark)
(Assignee)

Comment 2

2 years ago
Created attachment 8713466 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

This change eliminates a class of ESLint errors (camelcase), while
also bringing the method definitions in line with the MDN Coding
Style guidelines.
(Assignee)

Comment 3

2 years ago
Created attachment 8713471 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

This change eliminates a class of ESLint errors (camelcase), while
also bringing the method definitions in line with MDN Coding Style
guidelines.
(Assignee)

Updated

2 years ago
Attachment #8713466 - Attachment is obsolete: true
(Assignee)

Comment 4

2 years ago
Created attachment 8713473 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

This change eliminates a class of ESLint errors (camelcase), while
also bringing the method definitions in line with MDN Coding Style
guidelines.
(Assignee)

Updated

2 years ago
Attachment #8713471 - Attachment is obsolete: true
(Assignee)

Comment 5

2 years ago
Comment on attachment 8713473 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

Please also assign this bug to me. I didn't notice it was still unassigned until I went to submit the patch. Thanks!
Attachment #8713473 - Flags: review?(lclark)
Comment on attachment 8713473 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

Review of attachment 8713473 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good to me! The only problem is that the patch doesn't apply anymore because it's changing the same lines as you did with yesterday's patch (which has been committed). Once that has been resolved and the patch applies, I'll be happy to r+ it.
Attachment #8713473 - Flags: review?(lclark) → review-
(Assignee)

Comment 7

2 years ago
Created attachment 8713731 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

This change eliminates a class of ESLint errors (camelcase), while
also bringing the method definitions in line with MDN Coding Style
guidelines.
(Assignee)

Updated

2 years ago
Attachment #8713473 - Attachment is obsolete: true
(Assignee)

Comment 8

2 years ago
Comment on attachment 8713731 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

Well that was silly, I made merge conflicts solo. The updated patch should work a little better :).
Attachment #8713731 - Flags: review?(lclark)
(Assignee)

Updated

2 years ago
Assignee: nobody → ajkerrigan
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 8713731 [details] [diff] [review]
Remove named function syntax from webconsole.js methods

Review of attachment 8713731 [details] [diff] [review]:
-----------------------------------------------------------------

Great, thank you! 

I pushed to the try server to test it: https://treeherder.mozilla.org/#/jobs?repo=try&revision=8be86e84ccf1&selectedJob=16110679. None of the oranges seem related, so it looks good to me.
Attachment #8713731 - Flags: review?(lclark) → review+

Updated

2 years ago
Keywords: checkin-needed

Comment 11

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/7000943f09de
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox47: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 47
You need to log in before you can comment on or make changes to this bug.