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

Update: add ignoreDestructuring option to id-match rule #10554

Merged
merged 1 commit into from Nov 9, 2018

Conversation

tinymins
Copy link
Contributor

@tinymins tinymins commented Jul 4, 2018

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

add ignoreDestructuring option to id-match rule

Is there anything you'd like reviewers to focus on?

Nope.

Sorry that I do this code split job so late, cause I caught a cold and fever last week.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jul 4, 2018
@platinumazure platinumazure added enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Jul 4, 2018
Copy link
Member

@platinumazure platinumazure left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

One question: What is the current behavior of renamed and non-renamed import specifiers, and the intended behavior of the same after this pull request? (I'm wondering if we should have another option for import specifiers that works similarly for renamed import specifiers, maybe as a separate PR.)

@tinymins tinymins force-pushed the id-match-ignore-destructuring branch from 66eee77 to dff1a1c Compare July 5, 2018 01:43
@tinymins
Copy link
Contributor Author

tinymins commented Jul 5, 2018

@platinumazure I've recommitted, would you mind review again to see if there's any more problems should be fixed? Thanks a lot! :-)

@tinymins tinymins force-pushed the id-match-ignore-destructuring branch 3 times, most recently from 202ac79 to 0001128 Compare July 11, 2018 10:14
@platinumazure
Copy link
Member

Hi @tinymins, very sorry for losing track of this.

I'll champion this issue. (Will hopefully work with the ESLint team to accept this issue soon!)

I'll review this over the weekend!

@platinumazure platinumazure self-assigned this Sep 1, 2018
@tinymins
Copy link
Contributor Author

tinymins commented Sep 5, 2018

@platinumazure Thanks, with any problem in this PR, just point it out here, I'll check Github email everyday.

lib/rules/id-match.js Outdated Show resolved Hide resolved
tests/lib/rules/id-match.js Outdated Show resolved Hide resolved
lib/rules/id-match.js Show resolved Hide resolved
lib/rules/id-match.js Outdated Show resolved Hide resolved
docs/rules/id-match.md Show resolved Hide resolved
@tinymins tinymins force-pushed the id-match-ignore-destructuring branch 2 times, most recently from 73a390d to 68e7f95 Compare October 6, 2018 09:30
Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! just need one more 👍 to accept it. /cc @eslint/eslint-team

lib/rules/id-match.js Outdated Show resolved Hide resolved
lib/rules/id-match.js Outdated Show resolved Hide resolved
lib/rules/id-match.js Outdated Show resolved Hide resolved
@tinymins tinymins force-pushed the id-match-ignore-destructuring branch from 68e7f95 to 64d2e39 Compare October 10, 2018 14:29
Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tinymins!

@nzakas nzakas added accepted There is consensus among the team that this change meets the criteria for inclusion and removed evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion labels Nov 9, 2018
@nzakas nzakas merged commit c832cd5 into eslint:master Nov 9, 2018
@nzakas
Copy link
Member

nzakas commented Nov 9, 2018

Thanks @tinymins. Sorry it took so long to merge this in, but we really appreciate your work.

@tinymins
Copy link
Contributor Author

tinymins commented Nov 9, 2018

Thanks again for review and merge this!

@tinymins tinymins deleted the id-match-ignore-destructuring branch November 14, 2018 10:46
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators May 9, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion enhancement This change enhances an existing feature of ESLint rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants