Closed Bug 1174945 Opened 4 years ago Closed 4 years ago

enable eslint rules for Loop: no-shadow

Categories

(Hello (Loop) :: Client, defect)

defect
Not set

Tracking

(firefox41 fixed)

RESOLVED FIXED
mozilla41
Iteration:
41.3 - Jun 29
Tracking Status
firefox41 --- fixed

People

(Reporter: dmose, Assigned: andreio)

References

Details

Attachments

(1 file, 2 obsolete files)

http://eslint.org/docs/rules/no-shadow is what we'd like to enable.  Probably the easiest way to do this to just remove the "no-shadow": 0 line from .eslintrc, and allow it to revert to the default, which is:

{
    "rules": {
        "no-shadow": [2, {"hoist": "functions"}]
    }
}

Mark and Mike should feel free to chime in if they disagree...
Blocks: 1150279
Attached patch patch.diff (obsolete) — Splinter Review
Attached patch patch.diff (obsolete) — Splinter Review
Added in the commit message.
Attachment #8623169 - Attachment is obsolete: true
Attachment #8623172 - Flags: review?(dmose)
Comment on attachment 8623172 [details] [diff] [review]
patch.diff

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

Looks good, thanks for helping us avoid this pain in the future!  r=dmose with the changes requested here.

::: browser/components/loop/.eslintrc
@@ +50,5 @@
>      "no-multi-spaces": 0,         // TBD.
>      "no-new": 0,                  // TODO: Remove (use default)
>      "no-redeclare": 0,            // TODO: Remove (use default)
>      "no-return-assign": 0,        // TODO: Remove (use default)
> +    "no-shadow": [2, {"hoist": "functions"}],

I'd just delete this line entirely, so we don't inadvertently miss default values of future added options.

::: browser/components/loop/test/shared/feedbackViews_test.js
@@ +30,5 @@
>      sandbox.restore();
>    });
>  
>    // local test helpers
> +  function clickHappyFace(elem) {

This isn't really an element, it's a react component.  How about "component", here, and in similar changes.
Attachment #8623172 - Flags: review?(dmose) → review+
Attached patch patch.diffSplinter Review
Fixed review commments.
Attachment #8623172 - Attachment is obsolete: true
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/040a407b0705
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla41
Iteration: --- → 41.3 - Jun 29
Flags: firefox-backlog+
You need to log in before you can comment on or make changes to this bug.