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

Fix tests failing in master #2262

Merged
merged 1 commit into from
Jan 24, 2021
Merged

Fix tests failing in master #2262

merged 1 commit into from
Jan 24, 2021

Conversation

sdebarros
Copy link
Contributor

Tests in master are failing for the canary and beta versions, it seems to be related to the latest Ember not working well with a dependency of the ember-bootstrap we have.

This is a temporary solution to allow us to release version 3.1.0, since we only use ember-bootstrap for the test apps it shouldn't be an issue. Upgrading ember-bootstrap on the actual app, breaks tests for Ember 3.12. I think we should consider removing the ember-bootstrap dependency and just have our own CSS.

@sdebarros sdebarros marked this pull request as ready for review January 22, 2021 21:11
@sdebarros sdebarros requested a review from a team January 22, 2021 21:11
Copy link
Member

@marcoow marcoow left a comment

Choose a reason for hiding this comment

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

Makes sense 👍

I think we can also replace the dependency with a simple <link> tag to load the bootstrap styles from their CDN – I don't think we use any of the addon's additional functionality. Bootstrap as such might be a good thing to keep (for now) since moving to our own CSS would be a bit of a bigger step and we'd be adding more code to maintain that's not actually relevant for ESA.

@marcoow marcoow merged commit 74dd9cd into master Jan 24, 2021
@delete-merged-branch delete-merged-branch bot deleted the fix-master branch January 24, 2021 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants