-
Notifications
You must be signed in to change notification settings - Fork 167
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
feat(android, sdk12)!: update to facebook-android-sdk 12 #127
feat(android, sdk12)!: update to facebook-android-sdk 12 #127
Conversation
yarn was used everywhere else, and using `npm ci` required inclusion of a package-lock.json which could have different dependencies than yarn.lock this allows the use of one single yarn.lock, but the --frozen-lockfile argument on yarn install in combination with purging node_modules emulates `npm ci` behavior
this way we are in control of the version, npx may pull something in over the network, which is unwanted in CI / release where we need total control
also improve caching of yarn items by adding a version key, in case we need to forcibly remove caches in future
489cd7f
to
8d820e9
Compare
@thebergamo I'll let this sit for a day or two but it's all green in CI and passes locally so plan is to merge then release a breaking change release since an API was removed (both from iOS and Android SDKs, so removed here as well). It's a major advance for the library though to move forward on the underlying SDKs and update the example so I'm excited to get it out there |
I never seem so well documented PR! |
(just wanted to add that we are using this PR also for our app and as far as I can tell everything works 👍 Thank you very much for the work put in here!) |
Thanks very much for the success report! I'm just tidying up the example app generator right now but I can't see anything to change for the actual android forward port (it was really trivial anyway, thankfully) so when this actually merges+releases it won't really be a change for you to switch back to the release version Cheers |
…h-example.sh This includes all the current configuration items noted in app startup logs / native integration guides this improves the developer experience, all things noted or needed while I was working with the example to update the android SDK
BREAKING CHANGE: upstream SDKs removed updateUserProperties, there is no replacement
this proves Android targetSdk 31 works, when combined with facebook-android-sdk v12
note that we are not compatible with the new flow types, they get stricter every time. maintenance effort will be spent on typescript, with apologies feel free to post a PR that brings flow types up to v158 and re-enables the check in the CI test workflow
250d55e
to
235d19e
Compare
this will fix the build in CI for iOS at the same time it ensures we run CI against the actual development code (vs github master, which is what yarn installs by default) pod install will fail on ubuntu (when building android) so allow it to fail with the `|| true` bit, but we still install the dev code dependencies so android tests actual dev code
235d19e
to
a9506bd
Compare
Okay finally got this to go 100% green - nothing substantial in the module code has changed. |
@thebergamo I think we're ready for a release here but I wanted to coordinate with you to see if you have any thoughts just in case. It should be a major version, and I think this commit message is exactly what is needed to have that work automatically: The
What do you think? Ready for the release button? :-) |
I don't see any issues with it and I agree with you, it should be the fine as per the commit message. Go for it tiger 🐯 |
Great 🚀 🤞 |
🎉 This PR is included in version 5.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Great CI setup, it all worked perfectly! https://github.com/thebergamo/react-native-fbsdk-next/releases/tag/v5.0.0 |
This PR should be reviewed commit by commit and rebase-merged when ready. I'll keep the commits clean if they need changes, and re-push
Each commit has it's own message and is separate, for easy review
The big idea is:
Fixes #115
Related #55 (takes care of the android part)
Fixes #102
Test Plan:
I cleanly installed and ran example app on android emulator and ios simulator repeatedly while working through the upgrades, everything worked