Skip to content

Ember: Remove @embroider/test-setup Dependency #3460

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

Merged
merged 3 commits into from
Apr 27, 2021

Conversation

alexlafroscia
Copy link
Contributor

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).

The @embroider/test-setup package should be a development dependency rather than a "normal" one, since it's only used for testing purposes. The current state of the package means that the package is added to my own app when I install the @sentry/ember package, which I do not want.

I also noticed that @embroider/compat was listed as a dependency but never used, so I removed it. This was likely a holdover from the previous Embroider testing approach, before the test-setup package existed.

Running yarn install on master results in a considerable yarn.lock diff even without a package.json change, so I committed those changes separately but have included it in this PR so it's easier to see the actual yarn.lock impact of removing @embroider/compat.

The `yarn.lock` on `master` seems to be in an invalid state; these
changes happen by just running `yarn install` without even changing any
of the `package.json` files.
This package is only used for testing purposes, and should not be
a dependency of an Ember addon.
@kamilogorek kamilogorek merged commit 7335bee into getsentry:master Apr 27, 2021
@kamilogorek
Copy link
Contributor

Thanks!

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.

2 participants