Flavors documentation update#7867
Conversation
domesticmouse
left a comment
There was a problem hiding this comment.
This LGTM to me on the iOS side, but I'd like @jmagman's confirmation. I can't vouch for Android.
|
|
||
| ## Environment set up | ||
| Prerequisites: | ||
| * Xcode installed |
There was a problem hiding this comment.
Xcode is required for configuring Flavors for iOS.
Android Studio isn't strictly required for configuring Flavors for Android, but presumably is required for building an APK?
jmagman
left a comment
There was a problem hiding this comment.
I think you're missing instructions related to the ios/Podfile when users have a plugin. Flutter plugins complicate this flow, it would be worth adding a plugin to the example to show what to do.
Particularly, ios/Podfile needs to be updated from
project 'Runner', {
'Debug' => :debug,
'Profile' => :release,
'Release' => :release,
}
to
project 'Runner', {
'Debug-development' => :debug,
'Debug-production' => :debug,
'Profile-development' => :release,
'Profile-production' => :release,
'Release-development' => :release,
'Release-production' => :release,
}
8103a39 to
0e031cf
Compare
b42bde2 to
9630b36
Compare
MaryaBelanger
left a comment
There was a problem hiding this comment.
This is so well done! Some formatting suggestions to make it easier to follow
f9afdfa to
ac64d2e
Compare
MaryaBelanger
left a comment
There was a problem hiding this comment.
LGTM!
Left comments for some minor formatting inconsistencies you may or may not already be aware of, I think they're intentional anyway because formatting was rendering weirdly
sfshaza2
left a comment
There was a problem hiding this comment.
Some of the feedback from my previous review hasn't been implemented. Let me know if you want to meet.
|
I would like to land this before 2023. I think, once the edits are incorporated, it's good to go. Open an issue if there are any other "TODOs" that we want to incorporate. thx! |
sfshaza2
left a comment
There was a problem hiding this comment.
lgtm. @jmagman, if you still have issues with this page, either let @MiraMarshall know or file an issue. I believe she's addressed your issues.
jmagman
left a comment
There was a problem hiding this comment.
I think this would be a lot clearer if the screenshots showed a paid flavor next to free as we have in our sample app, since only having one flavor doesn't really make sense since in that case you could just update the default app without dealing with flavors.
However I know you want to get this merged ASAP. LGTM once my two last comments are addressed.
src/deployment/flavors.md
Outdated
| "request": "launch", | ||
| "type": "dart", | ||
| "program": "lib/main_development.dart", | ||
| "args": ["--flavor", "development", "--target", "lib/main_development.dart" ] |
There was a problem hiding this comment.
| "args": ["--flavor", "development", "--target", "lib/main_development.dart" ] | |
| "args": ["--flavor", "free", "--target", "lib/main_free.dart" ] |
src/deployment/flavors.md
Outdated
| Add the display name to **info.plist**. Create a new key in **info.plist** called **Bundle Display Name** | ||
| with the value `$(PRODUCT_NAME)`. |
There was a problem hiding this comment.
There should already be a Bundle Display Name key in that file (casing is Info.plist). How about:
| Add the display name to **info.plist**. Create a new key in **info.plist** called **Bundle Display Name** | |
| with the value `$(PRODUCT_NAME)`. | |
| Add the display name to **Info.plist**. Update the **Bundle Display Name** | |
| value to `$(PRODUCT_NAME)`. |
There was a problem hiding this comment.
Thanks Jenn! I've made the changes and will address the screenshots with #7979
Description of what this PR is changing or adding, and why:
Flavors staging site
This PR updates the flavors documentation.
Issues fixed by this PR (if any): Fixes #ISSUE-NUMBER
Presubmit checklist