-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
blueprints: remove explicit names from initializers. #14338
blueprints: remove explicit names from initializers. #14338
Conversation
The blueprint tests are failing (they assert the generated text). |
@rwjblue fixed now. |
@rwjblue friendly reminder 😉 |
@rwjblue ping |
I'm cool with this change. |
name: '<%= dasherizedModuleName %>', | ||
initialize | ||
}; | ||
export default { initialize }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we keep this multi-line to minimize diffs (here and for users, especially when adding before/afters)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, the diff will be just as small/large as before unless you start to introduce trailing commas 😉
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One-line diff:
export default {
initialize
}
->
export default {
after: 'ember-data',
initialize
};
vs
Four-line diff:
export default { initialize };
->
export default {
after: 'ember-data',
initialize
};
I also believe it'll be less confusing for new peeps if we keep it multi-line.
I don't have time to do this right now, but I believe you all should be able to edit my branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this seems like a reasonable change to me
+1 |
@locks @rwjblue @stefanpenner this seems ready to be merged. could someone press the big green button? |
See emberjs/guides#1654 and ember-cli/ember-cli-legacy-blueprints#54.