upgrade dependencies #126
upgrade dependencies #126
Conversation
|
@sandstrom Do you may have time for a review? I prefer to have another pair of eyes on my pull requests rather than merging them on my own. |
| @@ -12,6 +12,7 @@ | |||
| /.env* | |||
| /.eslintignore | |||
| /.eslintrc.js | |||
| /.git/ | |||
There was a problem hiding this comment.
this is ignored by default, no need to add:
https://docs.npmjs.com/using-npm/developers.html#keeping-files-out-of-your-package
There was a problem hiding this comment.
It was added to the addon blueprint in 3.13: ember-cli/ember-addon-output@7aea9e0#diff-0fd4ef892d9d4990033701887c2f9bcc
It was implemented in this commit: ember-cli/ember-cli@14ba465 The commit message says that at least in one case the /.git/ directory was not ignored by default. But it seemed to be unclear why.
| @@ -1,5 +1,5 @@ | |||
| 'use strict'; | |||
|
|
|||
| module.exports = { | |||
| extends: 'recommended' | |||
| extends: 'octane' | |||
There was a problem hiding this comment.
Don't know the implication of this, but I guess it only affect the dummy app? Should be same in that case.
There was a problem hiding this comment.
This should only affect template linting for this addon.
.travis.yml
Outdated
| - master | ||
| # npm version tags | ||
| - /^v\d+\.\d+\.\d+/ | ||
|
|
There was a problem hiding this comment.
Is there a reason why we're removing this?
There was a problem hiding this comment.
Ups. This was only required for testing my feature branch. Restored in 21a229c.
| } | ||
| }, | ||
| { | ||
| name: 'ember-default-with-jquery', |
There was a problem hiding this comment.
Do we need to test with jQuery? We're not using jQuery anywhere in this addon afaik. But if this is what "vanilla Ember" suggests on a default install, I guess we can keep it.
There was a problem hiding this comment.
It's part of the blueprint for addons: https://github.com/ember-cli/ember-addon-output/blob/master/config/ember-try.js But I agree that it shouldn't be needed for this addon. I don't care if we remove it or not.
|
@sandstrom Thanks a lot for your review. I hope I addressed all points. Let me know if you have follow up questions / concerns / change requests. If not, please feel free to squash and merge. |
This upgrades all dependencies to their latest versions and applying all changes in the blueprints that don't break the support policy (Ember 2.18+). If I recall correctly only the changes to native classes in dummy app are not taken from new blueprints cause that would require ember-native-class-polyfill only for testing.
Have done all the upgrades step by step after the hassle in #120. Feeling like RenovateBot. 😆 I have no idea why it has been that smooth this time to be honest.