Skip to content

Update @sentry/ember dependencies #5494

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

Closed
wants to merge 1 commit into from

Conversation

azhiv
Copy link

@azhiv azhiv commented Jul 29, 2022

Some of them are stale and are declared with ~, which restricts any minor
updates.
Also update README.md to require ember v3.24+ instead of v4+ because 3.24 is
already used for testing targets via ember-try.js.

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).

@azhiv
Copy link
Author

azhiv commented Jul 29, 2022

@k-fish Could you please take a look at this PR? The version of ember-auto-import is outdated and I will appreciate if you, guys, give an approve to update this dependency (along with some others as a drive-by change).

@azhiv azhiv marked this pull request as draft July 29, 2022 10:23
@azhiv azhiv marked this pull request as ready for review July 29, 2022 10:23
@azhiv azhiv force-pushed the update-ember-dependencies branch from b054b26 to 9a7a1c2 Compare July 29, 2022 10:39
@AbhiPrasad
Copy link
Member

Thanks for opening a PR! Think you'll need to update yarn.lock by running yarn!

@azhiv azhiv force-pushed the update-ember-dependencies branch from 9a7a1c2 to 4176b5f Compare July 29, 2022 15:51
@azhiv
Copy link
Author

azhiv commented Jul 29, 2022

Hey @AbhiPrasad, thanks for pointing that out. Including the amended yarn.lock.

@azhiv azhiv force-pushed the update-ember-dependencies branch from 4176b5f to a4b4536 Compare July 29, 2022 16:12
@azhiv
Copy link
Author

azhiv commented Jul 29, 2022

@AbhiPrasad I had to also update @embroider/test-setup dev dependency as it probably caused the tests to fail.

@azhiv azhiv force-pushed the update-ember-dependencies branch 3 times, most recently from 4ed4d72 to cb8ef7d Compare August 1, 2022 07:46
@azhiv
Copy link
Author

azhiv commented Aug 1, 2022

@AbhiPrasad I have to tag you one more time - I updated the default node version to be 12 mainly because v10 met its EOL on 30 Apr 2021 and it also failed the build for the new version of embroider.

Some of them are stale and are declared with ~, which restricts any minor
updates.
Also update README.md to require ember v3.24+ instead of v4+ because 3.24 is
already used for testing targets via `ember-try.js`.
@azhiv azhiv force-pushed the update-ember-dependencies branch from cb8ef7d to 34fde5d Compare August 2, 2022 08:57
@azhiv
Copy link
Author

azhiv commented Aug 2, 2022

@getsentry/open-source

@AbhiPrasad
Copy link
Member

Hey @azhiv! We unfortunately can’t accept this patch if we are bumping the default Node version. We only do this during major bumps to make sure folks don’t unexpectedly break.

See: https://develop.sentry.dev/sdk/philosophy/#compatibility-is-king

@azhiv
Copy link
Author

azhiv commented Aug 2, 2022

Understood, I'll try to proceed with node v10.

@azhiv
Copy link
Author

azhiv commented Aug 2, 2022

@AbhiPrasad Can you please guide me on how to test the build locally against node v10? With v12 yarn in the root folder succeeds, but when switching to v10, I get the following error:

 ~/src/sentry-javascript > update-ember-dependencies ±✚ |> yarn           
yarn install v1.22.19
[1/4] 🔍  Resolving packages...
[2/4] 🚚  Fetching packages...
warning Pattern ["[email protected]"] is trying to unpack in the same destination "/Users/artemz/Library/Caches/Yarn/v6/npm-react-router-6-6.3.0-3970cc64b4cb4eae0c1ea5203a80334fdd175557-integrity/node_modules/react-router-6" as pattern ["react-router-6@npm:[email protected]"]. This could result in non-deterministic behavior, skipping.
error @rollup/[email protected]: The engine "node" is incompatible with this module. Expected version ">=12.0.0". Got "10.24.1"
error Found incompatible module.
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.
 ✘ > ~/src/sentry-javascript > update-ember-dependencies ±✚ |> node -v
v10.24.1

After sorting this out, I'm going to run embroider-safe target which used to fail the build when using node v10. I guess I need to downgrade embrioder, but testing each version for compatibility via github actions is obviously an overkill so I'd rather prefer to perform the research locally.

@AbhiPrasad
Copy link
Member

Don't have the bandwidth to take a detailed look for the next couple days - but look at #5094 to understand our build system.

@rollup/plugin-sucrase shouldn't be used by ember - we build ember with the ember build system.

@github-actions
Copy link
Contributor

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@github-actions github-actions bot closed this Aug 31, 2022
@AbhiPrasad AbhiPrasad added the Package: ember Issues related to the Sentry Ember SDK label Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: ember Issues related to the Sentry Ember SDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants