search

expo / sentry-expo

MIT License
202 stars 83 forks source link

fix: warning on expo 46 #283

Closed stezu closed 1 year ago

stezu commented 2 years ago

Checklist

Why

Fixes an issue present with Expo 46 (also documented in #279) where the react.gradle import is now:

apply from: new File(reactNativeRoot, "react.gradle")

The previous regular expression was expecting a / character before the react.gradle string and was not matching this new import string.

How

The regular expression was updated to support both the old import style and the new one.

Test Plan

The change was used to run expo prebuild locally on a project upgraded to expo 46 and the build.gradle validated to ensure the sentry change was successfully applied.

kbrandwijk commented 2 years ago

Thanks for this PR. Can you also include the build output in your PR, as CI fails without. And if you want, can you also update your changelog entry to include the PR information, like the existing entries.

Thanks!

stezu commented 2 years ago

Thanks for this PR. Can you also include the build output in your PR, as CI fails without. And if you want, can you also update your changelog entry to include the PR information, like the existing entries.

Thanks!

@kbrandwijk I updated the changelog, but I'm not sure what you're looking for with the build output. The previous PRs to this repo don't appear to include build output in their descriptions so I don't have any examples to follow.

kbrandwijk commented 2 years ago

Thanks for this PR. Can you also include the build output in your PR, as CI fails without. And if you want, can you also update your changelog entry to include the PR information, like the existing entries. Thanks!

@kbrandwijk I updated the changelog, but I'm not sure what you're looking for with the build output. The previous PRs to this repo don't appear to include build output in their descriptions so I don't have any examples to follow.

If you run yarn build in the project, it will update the files in the build folder. Those are included in the repo.

justinkx commented 1 year ago

Thanks for this PR. Can you also include the build output in your PR, as CI fails without. And if you want, can you also update your changelog entry to include the PR information, like the existing entries. Thanks!

@kbrandwijk I updated the changelog, but I'm not sure what you're looking for with the build output. The previous PRs to this repo don't appear to include build output in their descriptions so I don't have any examples to follow.

If you run yarn build in the project, it will update the files in the build folder. Those are included in the repo.

@kbrandwijk Any update on this fix release?

kbrandwijk commented 1 year ago

Implemented in https://github.com/expo/sentry-expo/pull/290 due to lack of follow up here. Thank you for providing the fix though!