ESLint cleanup, batch #1

RESOLVED FIXED in Firefox 41

Status

()

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

People

(Reporter: ednapiranha, Assigned: ednapiranha)

Tracking

40 Branch
Firefox 41
Points:
---

Firefox Tracking Flags

(firefox41 fixed)

Details

Attachments

(1 attachment)

Comment hidden (empty)
(Assignee)

Comment 1

3 years ago
Created attachment 8626091 [details] [diff] [review]
Bug1177134.patch

First batch of tests. Let me know if anything should be adjusted.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=02acc9eacec5
Attachment #8626091 - Flags: review?(pbrosset)
Comment on attachment 8626091 [details] [diff] [review]
Bug1177134.patch

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

Thanks for working on this Jen.
The changes look good to me. I haven't spent too much time looking at the details but they look mechanical enough, and try is green.
r=me

::: browser/devtools/webconsole/test/browser_bug_638949_copy_link_location.js
@@ +80,5 @@
>    info("expected clipboard value: " + message.url);
>  
>    let deferred = promise.defer();
>  
> +  waitForClipboard((aData) => {

2 minor remarks:
- while we're at it, let's rename aData to data (https://wiki.mozilla.org/DevTools/CodingStandards#Code_style)
- parenthesis can be removed around aData since it's only 1 argument.

::: browser/devtools/webconsole/test/browser_cached_messages.js
@@ +18,5 @@
>  
>    loadTab(TEST_URI).then(testOpenUI);
>  }
>  
> +function testOpenUI(aTestReopen) {

Same remark about aArgument here.

::: browser/devtools/webconsole/test/browser_console_error_source_click.js
@@ +17,5 @@
>    loadTab(TEST_URI).then(() => {
>      HUDService.toggleBrowserConsole().then(browserConsoleOpened);
>    });
>  
> +  function browserConsoleOpened(aHud) {

Since you're changing this line, it's a good opportunity to rename aHud to hud.
We probably shouldn't use this bug to get rid of *all* aArgument notations, but I guess if you're already changing lines that have some declarations in them, it's a good chance to do this for them.
Attachment #8626091 - Flags: review?(pbrosset) → review+
(Assignee)

Updated

3 years ago
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/b026f257192e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox41: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.