-
Notifications
You must be signed in to change notification settings - Fork 41
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
Introduce "strip" and deprecate "environments" option #42
Conversation
No need to register it for the addon *and* the parent app
Actually, we don't replace it, but instead we deprecate the old option and introduce the new option on the side
var StripTestSelectorsTransform = require('./strip-test-selectors'); | ||
if (addonOptions.environments) { | ||
ui.writeDeprecateLine('The "environments" option in "ember-test-selectors" has been replaced ' + | ||
'with a "strip" option. Use e.g. "strip: EmberApp.env() === \'production\'" instead to ' + |
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.
"with the "strip" option"?
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.
my gut feeling says "with a "strip" option" sounds better, but I'm happy with either. maybe we should ask a native english speaker. 🤷♂️
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.
we should ask a native english speaker
👍 /cc @oliverbarnes, @sapeien
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.
the would be more correct in my opinion, because it matches the the from the beginning of the sentence.
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.
Concur about the
over a
.
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.
Updated. Thanks everyone!
Resolves #41