Skip to content
This repository has been archived by the owner on Jan 19, 2019. It is now read-only.

New: Add method decorators to AST (fixes #65) #64

Merged
merged 1 commit into from
Aug 24, 2016
Merged

New: Add method decorators to AST (fixes #65) #64

merged 1 commit into from
Aug 24, 2016

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented Aug 22, 2016

Closes #65

(Ignore the branch name! This PR relates to method decorators)

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@JamesHenry JamesHenry mentioned this pull request Aug 22, 2016
14 tasks
@JamesHenry
Copy link
Member Author

All in good time, my dear @eslintbot

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

var exp = convertChild(decorator.expression);
return {
type: "Decorator",
loc: exp.loc,
Copy link
Member

Choose a reason for hiding this comment

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

Based on Flow, we don't need this extra node.

@eslintbot
Copy link

Thanks for the pull request, @JamesHenry! I took a look to make sure it's ready for merging and found some changes are needed:

  • Pull requests with code require an issue to be mentioned at the end of the commit summary, such as (fixes #1234). Please update the commit summary with an issue (file a new issue if one doesn't already exist).

Can you please update the pull request to address these?

(More information can be found in our pull request guide.)

@eslintbot
Copy link

LGTM

@JamesHenry JamesHenry changed the title New: Process decorators on MethodDefinition node New: Add method decorators to AST (fixes #65) Aug 24, 2016
@eslintbot
Copy link

LGTM

@eslintbot
Copy link

LGTM

@JamesHenry
Copy link
Member Author

@nzakas Now ready for review, PTAL

@nzakas
Copy link
Member

nzakas commented Aug 24, 2016

This looks great, nice work!

@nzakas nzakas merged commit 7364cb9 into eslint:master Aug 24, 2016
@JamesHenry JamesHenry deleted the parameter-decorator-instance-member branch August 24, 2016 23:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants