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

Fix: no-obj-calls does not report errors for Reflect (fixes #7700) #7710

Merged
merged 1 commit into from Dec 9, 2016
Merged

Fix: no-obj-calls does not report errors for Reflect (fixes #7700) #7710

merged 1 commit into from Dec 9, 2016

Conversation

techeverri
Copy link
Contributor

@techeverri techeverri commented Dec 7, 2016

What is the purpose of this pull request?

[x] Bug fix

See #7700

What changes did you make?
I updated no-obj-calls to account for the new global Reflect introduced in ECMAScript 2015. This reports an error about the Reflect() call, because it's a global object property and not a function.

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

@mention-bot
Copy link

@techeverri, thanks for your PR! By analyzing the history of the files in this pull request, we identified @pedrottimark, @vitorbal and @ilyavolodin to be potential reviewers.

@jsf-clabot
Copy link

jsf-clabot commented Dec 7, 2016

CLA assistant check
All committers have signed the CLA.

@eslintbot
Copy link

LGTM

Copy link
Member

@not-an-aardvark not-an-aardvark 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 pull request!

The code changes look good to me, but would you mind making a couple changes to the commit message?

  • Since this is a bugfix that would cause more errors to be reported, the commit message should start with Update: instead of Fix:.
  • I think you might have forgotten the word Reflect at the end of the commit message (at the moment it's just no-obj-calls does not report errors for

Alternatively, we can update the commit message when we merge this pull request.

@not-an-aardvark not-an-aardvark added accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly rule Relates to ESLint's core rules labels Dec 7, 2016
@eslintbot
Copy link

LGTM

@techeverri techeverri changed the title Fix: no-obj-calls does not report errors for (fixes #7700) Fix: no-obj-calls does not report errors for Reflect (fixes #7700) Dec 7, 2016
@techeverri
Copy link
Contributor Author

@not-an-aardvark commit message updated :)

Copy link
Member

@not-an-aardvark not-an-aardvark left a comment

Choose a reason for hiding this comment

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

Thanks! This looks good to me.

We generally keep pull requests open for at least 2 days before merging them in case anyone else on the team has changes to suggest, so this will probably be merged sometime relatively soon unless anything unexpected comes up.

Copy link
Member

@mysticatea mysticatea left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@not-an-aardvark not-an-aardvark merged commit 4278c42 into eslint:master Dec 9, 2016
@not-an-aardvark
Copy link
Member

Thanks for contributing to ESLint!

@techeverri techeverri deleted the no-obj-call-add-reflect branch December 9, 2016 07:43
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 6, 2018
@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 Feb 6, 2018
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 bug ESLint is working incorrectly rule Relates to ESLint's core rules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants