-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
Add Codemagic App Previews (Android & iOS) for Pull Requests #257
Conversation
Visit the preview URL for this PR (updated for commit e6bdb84): https://sharezone-test--pr257-codemagic-app-previe-fbsx5wxw.web.app (expires Wed, 05 Oct 2022 23:13:25 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 |
⬇️ Generated builds by Codemagic for commit e6bdb84 ⬇️ Note: Only Sharezone team members are able to install the iOS app.
|
…pp/sharezone-app into codemagic-app-preview
Hey, this is just a quick thought written down, not a review yet. Since the So under which conditions does the I read this article today which talks about something similar: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ |
codemagic.yaml
Outdated
- certificate_credentials | ||
vars: | ||
BUNDLE_ID: de.codingbrain.sharezone.app | ||
flutter: v2.10.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this somehow be read from the fvm config or pubspec,yaml
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically, this should be possible, but it would take an overhead of ~1-2 minutes.
What Codemagic does:
- Download specified Flutter version
- Clone the repository
Therefore, we would need to this:
- Codemagic downloads the specified (or the default one) Flutter version
- Codemagic clones the repository
- We read out the Flutter version specified in the
fvm_config.json
- Download the Flutter version
- Update the environment variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm that kinda sucks. Do we have a document "update the flutter version" or sth where we can for now at least have the places written down where we would need to update the version string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using FVM seems to be a good option 👍 The overhead is roundabout 1 min
Good point about the security. I tried out to print a secret and it was easily possible. I asked the Codemagic support what you can to do to protect your secrets. |
…calablevideoview:v1.0.4-jitpack.`) (#302) Found this workaround here: TheWidlarzGroup/react-native-video#2454 (comment) Seems to work: https://codemagic.io/app/629c82ea463af7ff553fc7a5/build/63343dafb31705033692a7fa This PR is extracted from #257
@Jonas-Sander I added a few things. Can you re-review? You can simply use the feature "show changes since last review" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🙌
Co-authored-by: Jonas Sander <29028262+Jonas-Sander@users.noreply.github.com>
Description
We are already using the Deploy to Firebase action to deploy our web to a temporary url, when opening a new pull request.
codemagic_app_preview does the same for iOS and Android. Just scan the QR with your phone, install the app and review the changes 👍
Currently, I'm not able to build a iOS and macOS with one Codemagic workflow. Therefore, we are not building the macOS app at the moment. When I fixed this, I will make a new PR for adding support for macOS 👍
Google Sign In & Dynamic Links are not working at the moment for the Android version, because the dev Firebase project does not contain the SHA1 & SHA256 values of the code signing key (see #289).