-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Ember-cli upgrade from ~3.8 to ~3.20 #9972
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ll buttons without types
… question mark in nested get
…turing how to read the error
chelshaw
reviewed
Dec 3, 2020
chelshaw
reviewed
Dec 3, 2020
chelshaw
reviewed
Dec 3, 2020
chelshaw
reviewed
Dec 3, 2020
chelshaw
reviewed
Dec 3, 2020
@@ -24,19 +25,18 @@ export default Controller.extend({ | |||
|
|||
namespaceQueryParam: '', | |||
|
|||
/* eslint-disable-next-line ember/no-observers */ | |||
onQPChange: observer('namespaceQueryParam', function() { |
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 only needs to be an observer because it's based on a computed property, so we could potentially get away with computing on clusterController.namespaceQueryParam
which is what namespaceQueryParam is an alias of
chelshaw
reviewed
Dec 3, 2020
chelshaw
approved these changes
Dec 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR upgrades Ember from 3.8 to 3.22, effectively bringing us up to Octane. However, we do not octanify much in this PR other than the things required by the upgrade. This will allow us to develop/test before the 1.7 release to make sure no issues come up.
Used ember-cli-upgrade to run the update:
npm install -g ember-cli-update
ember-cli-update --to=~3.20
Note: Because the test suite appears to run faster we ran into a lot of race condition issues in the tests. Where we could solve for them we did, mostly adding in
settled()
. However, in the cases where we couldn't, we confirmed on the application they were not an issue and that individually the tests would pass. We then skipped the tests with notes/todos about coming back.Additionally, this upgrade upgraded a lot of other
devDependencies
per the recommendation of Embermaps upgrade article here.TODO's from within the code:
ember/no-new-mixins
&ember/no-mixins
) No fix for now{{navigate-input}}
CBSno-private-routing-service
CBS@secretType
is not defined ARG