Skip to content
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

ui: Dependency Upgrade #6387

Closed
wants to merge 4 commits into from

Conversation

johncowen
Copy link
Contributor

@johncowen johncowen commented Aug 23, 2019

This PR is ontop of our ember update to 3.8 and upgrades everything else (apart from ember-exam)

Interesting things of note:

  1. We switched all the new lint errors to warnings, we basically put off fixing these until a later date. Running the app now gives you lots of yellow warning messages to annoy, so they will be looked at fairly soon.
  2. We removed dart-sass which we weren't using but was left in there from an experiment from many moons ago 😮 !
  3. We removed a few other things which we don't use but are probably there from when we originally copied our starting skeleton from elsewhere.
  4. The jquery resolution is no longer needed, the upgrade pushes us into 3.4 (plus we hope to remove jquery entirely soon)
  5. We use a slightly custom href-to helper to globally fix wildcard URL encoding for ember wildcard routes (see UI: [BUGFIX] Decode/encode urls #5206). This needed altering slightly as a result of the upgrade, as a bonus we're aware that ember-href-to now supports sticky query params 🎉

We've left ember-exam without upgrading here potentially @alvin-huang might want to do that, or at least I need to check with him that the upgrade will be good for CI - probably to be done in a later PR.

@johncowen johncowen requested a review from a team August 23, 2019 17:39
@johncowen johncowen added the theme/ui Anything related to the UI label Aug 23, 2019
@@ -1,3 +1,6 @@
/*eslint ember/require-return-from-computed: "warn"*/
// TODO: Remove ^

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that you want to address these later but I think this one would be as simple as an else { return undefined; }

@alvin-huang
Copy link
Collaborator

I can follow up with you on the ember-exam upgrade, without testing the command myself and just looking at the latest releases, it looks like the current command in CircleCI should just work still.

@johncowen
Copy link
Contributor Author

@alvin-huang ah thanks for looking, thats cool then if it all looks good to you, I added a commit on the end here, everything continues to pass 🎉 , thankyou!

@johncowen johncowen changed the base branch from feature/ui-ember-update to feature/ui-ember-update-2 September 5, 2019 09:36
@johncowen johncowen changed the base branch from feature/ui-ember-update-2 to feature/ui-ember-update September 5, 2019 09:37
@johncowen johncowen changed the base branch from feature/ui-ember-update to ui-staging September 5, 2019 14:55
@johncowen johncowen changed the base branch from ui-staging to feature/ui-ember-update September 5, 2019 14:55
@johncowen
Copy link
Contributor Author

Closing in favour of #6452

@johncowen johncowen closed this Sep 5, 2019
@johncowen johncowen deleted the feature/ui-deps-upgrade branch September 12, 2019 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/ui Anything related to the UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants