Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

introduce the eslint prefer-const rule #16

Merged
merged 2 commits into from
Oct 27, 2016
Merged

Conversation

r-52
Copy link
Member

@r-52 r-52 commented Oct 26, 2016

Some files already use const, but some use let nearly exclusive. Eslint offers the prefer const which detects possible constant values. This PR introduces the rule and fixes all reported violations.

I've created two separated commits for:

  • introducing the rule
  • fixing the issues

Should I squash them together?

Some files already use `const`, but some use `let` nearly exclusive.
Eslint offers the `prefer const` which detects possible constant values.
This commit introduces the rule.
This commit fixes all the reported `prefer-const` violations .
"prefer-const": ["error", {
"destructuring": "any",
"ignoreReadBeforeAssign": false
}],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep definitely, this comes up all the time in code review for me artsy/emission#380 (review)

@orta
Copy link
Member

orta commented Oct 27, 2016

Hah - it was actually danger failing - "No Changelog changes!"

We still don't have any feedback yet, was planning on starting to look at that today. I'm OK with this being red for now 👍

I also don't care about git history - so this is great

@orta orta merged commit 2b82a47 into danger:master Oct 27, 2016
@orta
Copy link
Member

orta commented Oct 27, 2016

Thanks @romankl - would you like to join the Danger org? we conform to the Moya Community Continuity - you're welcome to participate in PRs and help out at a level that works for you.

@r-52
Copy link
Member Author

r-52 commented Oct 27, 2016

@orta hey, whoaa that would be an honor - thank you! 👍 (right now it was just a small PR)

@r-52 r-52 deleted the eslint-const-rule branch October 27, 2016 14:35
@orta
Copy link
Member

orta commented Oct 27, 2016

sent - no problem, gives you more freedom, and eventually we'll add a Danger rule to ask, https://github.com/danger/danger/blob/master/Dangerfile#L14-L22

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants