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

Build and test all apps with embroider #7685

Merged
merged 6 commits into from
Mar 6, 2024

Conversation

jrjohnson
Copy link
Member

Added CI jobs to build and test and the infrastructure to do this and then cleaned up the relationship with ember-data in all the apps by moving it to dependencies of ilios-common and removing it from each app. This lets embroider handle making it available correctly.

@jrjohnson jrjohnson force-pushed the embroidery-goodness branch from 39d4975 to 9f5f394 Compare March 5, 2024 00:57
Added CI jobs to build and test and the infrastructure to do this and
then cleaned up the relationship with ember-data in all the apps by
moving it to dependencies of ilios-common and removing it from each app.
This lets embroider handle making it available correctly.
@jrjohnson jrjohnson force-pushed the embroidery-goodness branch from 9f5f394 to 9843d7d Compare March 6, 2024 16:53
Even when there are failures we want to see the results of all of these
tests.
This is a leftover from ember a while ago, it doesn't seem to be
necessary and it was breaking embroider.
This is the recommended configuration for the best dependency structure,
however it doesn't work for us today. I'm leaving it in with the options
we want commented out to, hopefully, give us a target to push towards.
Something is up with the latest ember-data release so let's pin here for
now.
Without this the babel transform doesn't get resolved in an embroider
build.
@jrjohnson jrjohnson marked this pull request as ready for review March 6, 2024 19:35
@jrjohnson jrjohnson requested a review from stopfstedt March 6, 2024 19:35
Copy link
Member

Choose a reason for hiding this comment

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

great idea. i appreciate the extra context in the commit message that comes with this as well.

@@ -25,10 +25,7 @@ module.exports = function (defaults) {

hinting: isTestBuild,
babel: {
plugins: [
// eslint-disable-next-line
Copy link
Member

Choose a reason for hiding this comment

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

interesting. so eslint was onto something here after all.

Copy link
Member

@stopfstedt stopfstedt left a comment

Choose a reason for hiding this comment

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

good stuff.

@dartajax
Copy link
Member

dartajax commented Mar 6, 2024

Build (20) Expected - is it stalled out?

@stopfstedt
Copy link
Member

Build (20) Expected - is it stalled out?

this step was part of the previous pipeline. i removed it.

@dartajax dartajax merged commit f212c0d into ilios:master Mar 6, 2024
26 checks passed
@jrjohnson jrjohnson deleted the embroidery-goodness branch March 6, 2024 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants