-
Notifications
You must be signed in to change notification settings - Fork 32
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
Initial commit for ember-cli 2.15.0 support #33
Conversation
@SergeAstapov ready for review when you are ready. |
index.js
Outdated
* and needed to be moved into `init`. | ||
*/ | ||
init() { | ||
this._super && this._super.apply(this, arguments); |
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.
@jasonmit this line seems to be failing on CI. Could you please change to this._super.init && this._super.init.apply(this, arguments);
as per docs https://ember-cli.com/api/classes/Addon.html#method_init
package.json
Outdated
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "ember-cli-dotenv", | |||
"version": "1.2.0", | |||
"version": "2.0.0", |
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.
@jasonmit do you mind to keep it 1.2.0
for now and bump to 2.0 separately? I'd like to add FastBoot support in 2.0 as well
@SergeAstapov good feedback, I've addressed the concerns. Thanks. |
# Ember CLI Dotenv | ||
|
||
# Installation | ||
|
||
`ember install ember-cli-dotenv` | ||
|
||
# Upgrading to 2.0.0 |
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.
Unsure how we want to deal with this, are you planning to land your fastboot work soon or should I defer the README changes until after we version? @SergeAstapov
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.
I do actually have a branch with FastBoot support, but it appeared there is a small issue in ember-cli-fastboot that blocks this addon to have FastBoot support, see ember-fastboot/ember-cli-fastboot#543
To not block people from using this addon I think we can
- merge this PR
- merge PR with ember-cli update
- fix tests - I can do this tomorrow
- update readme and release 2.0
- add FastBoot support
- release 2.1
What about such plan?
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.
Sounds good to me!
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.
@jasonmit I'll take care of ember-cli upgrade (need to resolve conflict) and failing tests so we would be able to release version 2.0 shortly
CI has been failing for awhile, we should address in a follow up PR. Likely can be addressed when we upgrade ember-cli. First time it started failing a few years ago with the error we are still seeing today: https://travis-ci.org/fivetanley/ember-cli-dotenv/jobs/116980605 |
No description provided.