1243764 - Fix camelcase warnings in webconsole.js

Status: RESOLVED FIXED

(DevTools :: Console, defect)

Description

[AJ Kerrigan [:ajkerrigan]]

+++ 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.

Comment 1

[Lin Clark [:linclark]]

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.

Comment 2

[AJ Kerrigan [:ajkerrigan]]

Attachment: 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.

Comment 3

[AJ Kerrigan [:ajkerrigan]]

Attachment: 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.

Comment 4

[AJ Kerrigan [:ajkerrigan]]

Attachment: 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.

Comment 5

[AJ Kerrigan [:ajkerrigan]]

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!

Comment 6

[Lin Clark [:linclark]]

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.

Comment 7

[AJ Kerrigan [:ajkerrigan]]

Attachment: 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.

Comment 8

[AJ Kerrigan [:ajkerrigan]]

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 :).

Comment 9

[Lin Clark [:linclark]]

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.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/7000943f09de

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/7000943f09de