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: Fix embroider optimized CI builds #1591

Merged
merged 3 commits into from
Aug 16, 2023

Conversation

HeroicEric
Copy link
Contributor

@HeroicEric HeroicEric commented Aug 15, 2023

This fixes the embroiderOptimized ember-try scenario that runs as part of CI mostly by removing the dependency on other addons that cause issues:

  • Removes ember-cli-fastboot
  • Removes ember-cli-deprecation-workflow
  • Removes ember-fetch

Removing ember-cli-fastboot

This removes ember-cli-fastboot because it is not currently compatible with embroider optimized builds.

I believe ember-cli-fastboot was added as a dependency so ember-power-select could run some tests to ensure compatibility. This makes sense but AFAICT the tests to check fastboot compatibility have not actually been running since 2016. Since these fastboot tests have not actually been running for so long, IMO it makes sense to remove all of this for now and revisit fastboot tests separately.

Here is the commit where the command to run the fastboot tests was removed 1b9f90d#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L13

Removing ember-cli-deprecation-workflow

There's an issue with ember-cli-deprecation-workflow that causes an error during CI for the embroider optimized build.

I think we can remove ember-cli-deprecation-workflow from this addon because it does not seem to be doing anything useful but maybe I'm missing something?

This was originally added in c10dd8e as part of #1431 but there is not really much context to go off of as far as why it was added. It seems like maybe it was added while debugging GitHub Actions and accidentally left laying around?

Removing ember-fetch

This also removes the direct dependency on ember-fetch because it was only used in the docs/tests and causes some issues related to embroider.

The issue was reported here ember-cli/ember-fetch#622 and the fix was basically to get around the issue by using native promises rather than RSVP. This root of the issue was not actually fixed yet and here is the open issue embroider-build/embroider#1195

@HeroicEric HeroicEric marked this pull request as draft August 15, 2023 20:44
@cibernox
Copy link
Owner

is this ready?

There's [an issue with ember-cli-deprecation-workflow](ember-cli/ember-cli-deprecation-workflow#133)
that causes an error during CI for the embroider optimized build.

I think we can remove `ember-cli-deprecation-workflow` from this addon
because it does not seem to be doing anything useful but maybe I'm
missing something?

This was originally added in
cibernox@c10dd8e
as part of cibernox#1431 but
there is not really much context to go off of as far as why it was
added. It seems like maybe it was added while debugging GitHub Actions
and accidentally left laying around?

- ember-cli/ember-cli-deprecation-workflow#133
- https://github.com/embroider-build/embroider/tree/main/packages/test-setup#embroideroptimizedembertryconfig
@HeroicEric HeroicEric marked this pull request as ready for review August 16, 2023 19:37
@HeroicEric
Copy link
Contributor Author

@cibernox should be ready now

This removes `ember-cli-fastboot` because it is not currently compatible
with embroider optimized builds.

I believe `ember-cli-fastboot` was added as a dependency so
`ember-power-select` could run some tests to ensure compatibility. This
makes sense but AFAICT the tests to check fastboot compatibility have
not actually been running since 2016. Since these fastboot tests have not
actually been running for so long, IMO it makes sense to remove all of
this for now and revisit fastboot tests separately.

- Stopped running fastboot tests cibernox@1b9f90d#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519L13
@cibernox cibernox merged commit d6eb0f0 into cibernox:master Aug 16, 2023
16 checks passed
@cibernox
Copy link
Owner

thanks!

@HeroicEric HeroicEric deleted the fix-build branch August 16, 2023 20:20
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.

3 participants