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

Refactor {{mount}} syntax #14967

Merged
merged 2 commits into from
Mar 2, 2017
Merged

Conversation

mike183
Copy link
Contributor

@mike183 mike183 commented Feb 27, 2017

Supersedes ember-engines/ember-engines#338

This PR adds no new functionality and is purely a refactor of the {{mount}} syntax in order to lay the ground work required for:

cc/ @rwjblue

this.nameRef = nameRef;
this.env = env;
this.symbolTable = symbolTable;
this.args = args;
Copy link
Member

Choose a reason for hiding this comment

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

this seems to be unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

}

this._lastName = nameOrDef;
this._lastDef = new MountDefinition(nameOrDef, env);
Copy link
Member

Choose a reason for hiding this comment

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

Remove env from MountDefinition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed

Copy link
Member

@chancancode chancancode left a comment

Choose a reason for hiding this comment

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

LGTM other than the two small issues

@mike183
Copy link
Contributor Author

mike183 commented Mar 2, 2017

Thanks for the review, I believe I have resolved all of the issues that were raised.

@rwjblue
Copy link
Member

rwjblue commented Mar 2, 2017

Awesome! 👏

@rwjblue rwjblue merged commit 4d0a17b into emberjs:master Mar 2, 2017
@rwjblue
Copy link
Member

rwjblue commented Mar 2, 2017

Next up: allowing bound engine names?

@mike183 mike183 deleted the mount-syntax-refactor branch March 2, 2017 12:11
@mike183
Copy link
Contributor Author

mike183 commented Mar 2, 2017

Thanks!

Yep, bound engine names next.

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.

3 participants