Upgrade to latest ember versions & fix all warnings/errors #157
Conversation
|
@chrislopresto Please let me know if this PR needs improvement. |
|
@sivakumar-kailasam This PR looks great, thanks! What codemod did you use? We just finally merged #100 which was pretty big, so this is going to need a rebase. If you have some time over the next few days, great. If not, no worries... I could hop in to resolve the merge conflicts. Thoughts? |
|
@chrislopresto I used ember-cli-update for the upgrade. I also used ember-modules-codemod for import rewrites on rebase conflicts. Rebased against latest changes in master. Hope this helps |
There was a problem hiding this comment.
Thanks so much for doing this! I hadn't fully wrapped my head around some of resolutions to the ember linter errors... thanks for demonstrating the way.
I left some comments, mainly centering around the ember-try config. Let me know if you don't have time to tweak and I can pick up when I'm working on things this week.
Cheers!
.travis.yml
Outdated
| @@ -25,11 +25,8 @@ env: | |||
| matrix: | |||
| # we recommend new addons test the current and previous LTS | |||
| # as well as latest stable release (bonus points to beta/canary) | |||
| - EMBER_TRY_SCENARIO=ember-lts-2.4 | |||
| - EMBER_TRY_SCENARIO=ember-lts-2.8 | |||
There was a problem hiding this comment.
For the time being, I'd like to leave 2.4 and 2.8 in the ember-try config so we remain aware of freestyle's compatibility in CI. We're only officially supporting the trailing 2 LTS releases (2.16 and 2.18 technically), so we'll include 2.4 and 2.8 in the allow_failures section below. Perhaps we should also add 2.12 as an allowed failure while we're at it.
.travis.yml
Outdated
| - EMBER_TRY_SCENARIO=ember-lts-2.12 | ||
| - EMBER_TRY_SCENARIO=ember-lts-2.16 | ||
| - EMBER_TRY_SCENARIO=ember-lts-2.18 |
There was a problem hiding this comment.
Since 2.18 is an LTS release, let's leave this in.
.travis.yml
Outdated
| @@ -38,8 +35,6 @@ env: | |||
| matrix: | |||
| fast_finish: true | |||
| allow_failures: | |||
| - env: EMBER_TRY_SCENARIO=ember-lts-2.4 | |||
| - env: EMBER_TRY_SCENARIO=ember-lts-2.8 | |||
There was a problem hiding this comment.
These would come back in, too.
| @@ -19,15 +19,15 @@ export default Ember.Component.extend({ | |||
| actions: { | |||
| toggleCheckbox() { | |||
| let newValue = !this.get('value'); | |||
| this.attrs.changeValueTo(newValue); | |||
config/ember-try.js
Outdated
| module.exports = { | ||
| useYarn: true, |
There was a problem hiding this comment.
I think we should keep this useYarn flag, if only for speed. 🙂
config/ember-try.js
Outdated
| 'ember-source': null | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
These 2.4, 2.8, and 2.18 entries would come back as well.
jsconfig.json
Outdated
| @@ -0,0 +1 @@ | |||
| {"compilerOptions":{"target":"es6","experimentalDecorators":true},"exclude":["node_modules","bower_components","tmp","vendor",".git","dist"]} | |||
There was a problem hiding this comment.
This file comes from VS Code, right? Should we gitignore it?
package.json
Outdated
| "broccoli-merge-trees": "^2.0.0", | ||
| "broccoli-writer": "^0.1.1", | ||
| "ember-cli-babel": "^6.6.0", | ||
| "ember-cli-htmlbars": "^2.0.3", | ||
| "ember-cli-sass": "^6.1.3", | ||
| "ember-cli-sass": "^7.1.4", |
There was a problem hiding this comment.
No need for any change in this PR... just calling this out for posterity:
Some folks have been having problems with ember-cli-sass dependency resolution based on their local app's setup. I believe ember-cli-sass 7 (released in July) has some breaking changes from ember-cli-sass 6, so I don't know if this upgrade will help or hurt. Regardless, I'm going to change ember-freestyle's scss to vanilla CSS soon, so I say we roll with this as is for the time being.
| @@ -1,52 +0,0 @@ | |||
| { | |||
|
@chrislopresto addressed all the review comments with the latest commit. Should be gtg :) |
|
https://github.com/ember-cli/eslint-plugin-ember/tree/master/docs/rules is a good place to learn the reasoning behind each of the rules brought in by that plugin. Its well documented. |
|
Thanks! |
|
@chrislopresto can you please publish a new version? I have a PR on another repo that was waiting on this one. |
Used the codemod to migrate to latest version. Then fixed eslint errors & ember-cli-page-object warnings.