Add documentation about need tools and versions to build our app #963
Conversation
KarinBerg
requested review from
ssandino,
andrashee,
mkue,
novas1r1 and
MDikkii
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several changes to the Changes
Warning Rate limit exceeded@andrashee has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 14 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (8)
recipients_app/clean_build.sh (1)
6-6: Add newline at end of fileAdd a newline at the end of the file to follow Unix file conventions.
flutter gen-l10n +recipients_app/docu/app_release_guides/releasing.md (1)
1-1: Add Flutter version requirementSince this PR introduces Flutter 3.19.6, it's important to mention the required Flutter version before starting the release process.
# How to release the apps +Prerequisites: +- Flutter 3.19.6 (specified in .tool-versions) + 1. Increment app version and build number in pubspec.yamlrecipients_app/README.md (6)
5-5: Fix article usageChange "a Apple" to "an Apple" for correct grammar.
-## Tools needed for building the app on a Apple Silicon Mac +## Tools needed for building the app on an Apple Silicon Mac🧰 Tools
🪛 LanguageTool
[misspelling] ~5-~5: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...## Tools needed for building the app on a Apple Silicon Mac - [Homebrew](https://...(EN_A_VS_AN)
63-63: Fix grammar and redundancyThe sentence has incorrect article usage and redundant wording.
-Open an terminal inside of vsCode +Open a terminal in vsCode🧰 Tools
🪛 LanguageTool
[misspelling] ~63-~63: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ts_appproject folder in vsCode - Open an terminal inside of vsCode and checkfl...(EN_A_VS_AN)
[style] ~63-~63: This phrase is redundant. Consider using “inside”.
Context: ...ect folder in vsCode - Open an terminal inside of vsCode and checkflutter --versionis...(OUTSIDE_OF)
59-61: Add more context about Sentry configurationThe instructions about Sentry URL lack context about where to obtain it.
- - Replace the value "FILL IN SENTRY URL" after "SENTRY_URL=" with the real Sentry url to be able to use Sentry + - Replace the value "FILL IN SENTRY URL" after "SENTRY_URL=" with the Sentry DSN URL + - You can find the Sentry DSN URL in your Sentry project settings under Client Keys (DSN)
115-124: Enhance version upgrade instructionsThe Flutter version upgrade section could benefit from additional validation steps.
### Upgrade flutter version If you upgrade the flutter version, you have to change the following locations as well: - pubspec.yaml - Under 'environment' adjust the 'sdk' and 'flutter' versions - .tool-versions (version file for version manager ASDF) - If you use 'asdf' run the comman `asdf local flutter x.y.z` #Replace x.y.z with the new Flutter version. - Otherwise just update the version number in the file - README.md - Find all places in the README.md which mentions the Flutter version number +- After updating: + - Run `flutter doctor` to verify the installation + - Run `flutter pub get` to update dependencies + - Run all tests to ensure compatibility🧰 Tools
🪛 LanguageTool
[uncategorized] ~121-~121: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
43-56: Fix markdown formatting in asdf sectionThe asdf section has incorrect list indentation and missing code block language specifications.
## Optionally: Use the version manager [asdf](https://asdf-vm.com/) - ``` +```bash export FLUTTER_ROOT="$(asdf where flutter)" ### asdf stuff ############ source $(brew --prefix asdf)/libexec/asdf.sh export ASDF_NODEJS_LEGACY_FILE_DYNAMIC_STRATEGY=latest_available # This is optional. It installs tools defined in .tool-versions on terminal start asdf install ########################### - ``` +```🧰 Tools
🪛 Markdownlint (0.35.0)
44-44: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
45-45: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
46-46: Expected: 0; Actual: 1
Unordered list indentation(MD007, ul-indent)
43-43: null
Headings must start at the beginning of the line(MD023, heading-start-left)
47-47: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
64-64: Add context about clean_build.sh scriptThe instruction to run
clean_build.shlacks explanation of its purpose.-Run `./clean_build.sh` +Run `./clean_build.sh` to clean the build cache and rebuild the project from scratch +# This script will: +# 1. Clean Flutter build cache +# 2. Get dependencies +# 3. Generate necessary files
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
recipients_app/.tool-versions(1 hunks)recipients_app/.vscode/launch.json.example(1 hunks)recipients_app/README.md(3 hunks)recipients_app/clean_build.sh(1 hunks)recipients_app/docu/app_release_guides/releasing.md(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- recipients_app/.tool-versions
- recipients_app/.vscode/launch.json.example
🧰 Additional context used
🪛 LanguageTool
recipients_app/docu/app_release_guides/releasing.md
[grammar] ~4-~4: It appears that a hyphen is missing in this expression.
Context: ...y 1. Incrementing the version code is a must have for a new prod version. Otherwise the s...
(MUST_HAVE)
[uncategorized] ~4-~4: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ... is a must have for a new prod version. Otherwise the stores will not accept the new app....
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
recipients_app/README.md
[misspelling] ~5-~5: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...## Tools needed for building the app on a Apple Silicon Mac - [Homebrew](https://...
(EN_A_VS_AN)
[duplication] ~26-~26: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~28-~28: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~35-~35: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ... flutter config --jdk-dir $JAVA_HOME. Otherwise you will get the error "Unsupported cla...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[misspelling] ~63-~63: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ts_appproject folder in vsCode - Open an terminal inside of vsCode and checkfl...
(EN_A_VS_AN)
[style] ~63-~63: This phrase is redundant. Consider using “inside”.
Context: ...ect folder in vsCode - Open an terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~80-~80: Possible missing comma found.
Context: ...son / plist file (not in the repo) For development use the dev or stage flavor. As Fi...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~121-~121: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint (0.35.0)
recipients_app/README.md
44-44: Expected: 0; Actual: 1
Unordered list indentation
(MD007, ul-indent)
45-45: Expected: 0; Actual: 1
Unordered list indentation
(MD007, ul-indent)
46-46: Expected: 0; Actual: 1
Unordered list indentation
(MD007, ul-indent)
43-43: null
Headings must start at the beginning of the line
(MD023, heading-start-left)
15-15: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
20-20: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
29-29: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
47-47: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
94-94: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
98-98: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 Shellcheck (0.10.0)
recipients_app/clean_build.sh
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (6)
recipients_app/README.md (6)
5-5: Fix article usageChange "a Apple" to "an Apple" for correct grammar.
-## Tools needed for building the app on a Apple Silicon Mac +## Tools needed for building the app on an Apple Silicon Mac🧰 Tools
🪛 LanguageTool
[misspelling] ~5-~5: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...## Tools needed for building the app on a Apple Silicon Mac - [Homebrew](https:/...(EN_A_VS_AN)
17-19: Add language specifiers to code blocksAdd shell language specifier to code blocks for proper syntax highlighting.
- ``` + ```shellAlso applies to: 23-27, 35-38
🧰 Tools
🪛 Markdownlint (0.35.0)
17-17: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
41-44: Fix punctuation after conjunctive adverbsAdd commas after "Otherwise" for proper grammar.
-Otherwise you will get the error +Otherwise, you will get the error🧰 Tools
🪛 LanguageTool
[uncategorized] ~42-~42: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...flutter config --jdk-dir $JAVA_HOME. Otherwise you will get the error "Unsupported...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
59-67: Add language specifier to code blockAdd shell language specifier to the code block.
- ``` + ```shell🧰 Tools
🪛 Markdownlint (0.35.0)
59-59: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
80-80: Fix article usageChange "an terminal" to "a terminal" for correct grammar.
-Open an terminal inside of vsCode +Open a terminal inside vsCode🧰 Tools
🪛 LanguageTool
[misspelling] ~80-~80: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ts_appproject folder in vsCode - Open an terminal inside of vsCode and checkfl...(EN_A_VS_AN)
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ect folder in vsCode - Open an terminal inside of vsCode and checkflutter --versionis...(OUTSIDE_OF)
101-101: Add missing commaAdd a comma after "For development" for proper grammar.
-For development use the `dev` or `stage` flavor. +For development, use the `dev` or `stage` flavor.🧰 Tools
🪛 LanguageTool
[uncategorized] ~101-~101: Possible missing comma found.
Context: ...son / plist file (not in the repo) For development use thedevorstageflavor. As Fi...(AI_HYDRA_LEO_MISSING_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipients_app/README.md(4 hunks)recipients_app/docu/app_release_guides/releasing.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
recipients_app/README.md
[misspelling] ~5-~5: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...## Tools needed for building the app on a Apple Silicon Mac - [Homebrew](https:/...
(EN_A_VS_AN)
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~42-~42: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ... flutter config --jdk-dir $JAVA_HOME. Otherwise you will get the error "Unsupported...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[misspelling] ~80-~80: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ts_appproject folder in vsCode - Open an terminal inside of vsCode and checkfl...
(EN_A_VS_AN)
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ect folder in vsCode - Open an terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~101-~101: Possible missing comma found.
Context: ...son / plist file (not in the repo) For development use the dev or stage flavor. As Fi...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~149-~149: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
recipients_app/docu/app_release_guides/releasing.md
[grammar] ~6-~6: It appears that a hyphen is missing in this expression.
Context: ... Incrementing the version code is a must have for a new prod version. Otherwise ...
(MUST_HAVE)
[uncategorized] ~7-~7: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ... must have for a new prod version. Otherwise the stores will not accept the new app....
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🪛 Markdownlint (0.35.0)
recipients_app/README.md
17-17: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
23-23: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
35-35: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
59-59: null
Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🔇 Additional comments (2)
recipients_app/docu/app_release_guides/releasing.md (1)
4-7: Fix grammar and improve clarity with examples
The instruction needs refinements for better clarity and correctness.
- - Open 'pubspec.yaml' and increase 'version' with the desired app
- version and the version code by incrementing the '+' part by 1.
- Incrementing the version code is a must have for a new prod
- version. Otherwise the stores will not accept the new app.
+ - Open 'pubspec.yaml' and increase 'version' with the desired app version and the version code by incrementing the '+' part by 1. Incrementing the version code is a must-have for a new prod version. Otherwise, the stores will not accept the new app.
+
+ Example:
+ Current: version: 1.1.0+12
+ New: version: 1.1.1+13🧰 Tools
🪛 LanguageTool
[grammar] ~6-~6: It appears that a hyphen is missing in this expression.
Context: ... Incrementing the version code is a must have for a new prod version. Otherwise ...
(MUST_HAVE)
[uncategorized] ~7-~7: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ... must have for a new prod version. Otherwise the stores will not accept the new app....
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
recipients_app/README.md (1)
24-24: Remove hardcoded user path
The path contains a hardcoded username which should be replaced with a placeholder.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipients_app/clean_build.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
recipients_app/clean_build.sh
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
🔇 Additional comments (3)
recipients_app/clean_build.sh (3)
5-12: LGTM: Robust error handling implementation
The error handling with set -e and Flutter installation check is well implemented.
1-26: Add executable permissions to the script
The script needs to be executable to run properly.
Run this command:
chmod +x recipients_app/clean_build.sh🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
14-26: Verify Flutter version compatibility
The script looks good with clear progress feedback. Let's verify it works with the specified Flutter version.
✅ Verification successful
Flutter version and build tools are properly configured
The clean_build.sh script is compatible with the project's Flutter configuration:
- Flutter SDK version 3.19.6 is specified in pubspec.yaml
- Required dependencies for build_runner and l10n generation are present
- All commands in the script are standard Flutter CLI commands supported across versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the Flutter version in .tool-versions matches the one required
rg -l "flutter.*3.19.6" .tool-versions
# Check for any version-specific Flutter commands in the codebase
rg "flutter.*gen-l10n|build_runner" --type dart
Length of output: 145
Script:
#!/bin/bash
# Check for Flutter SDK version specification in pubspec.yaml
rg "sdk:|flutter:" pubspec.yaml
# Check for build_runner and l10n configurations
fd pubspec.yaml --exec cat {}
Length of output: 1887
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (4)
recipients_app/README.md (4)
64-66: Consider warning about automatic installation behaviorThe automatic
asdf installon terminal start might be unexpected and could slow down terminal startup. Consider making this optional or adding a warning note.
75-76: Add more context about Sentry URLPlease provide information about where to obtain the Sentry URL and why it's needed.
77-78: Fix grammar in flavor selection instructionChange "what to use" to "want to use".
- - Decide which flavor and backend environment you what to use and + - Decide which flavor and backend environment you want to use and🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: Possible missing preposition found.
Context: ...Sentry url to be able to use Sentry - Decide which flavor and backend environment yo...(AI_HYDRA_LEO_MISSING_TO)
141-152: Consider automating version updatesConsider creating a script to automatically update Flutter version references across all files to prevent inconsistencies.
Would you like me to help create an automation script for version updates?
🧰 Tools
🪛 LanguageTool
[uncategorized] ~149-~149: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipients_app/README.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
recipients_app/README.md
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~77-~77: Possible missing preposition found.
Context: ...Sentry url to be able to use Sentry - Decide which flavor and backend environment yo...
(AI_HYDRA_LEO_MISSING_TO)
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~149-~149: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (4)
recipients_app/README.md (4)
5-12: LGTM! Clear and comprehensive tool requirements.
The tools section effectively lists all required dependencies with specific versions.
24-25: Remove hardcoded user path
The path contains a hardcoded username "karinberg" which should be replaced with a placeholder.
-export ANDROID_HOME="/Users/karinberg/Development/Android/SDK"
+export ANDROID_HOME="$HOME/Development/Android/SDK"89-101: LGTM! Clear flavor documentation.
The flavors section effectively explains the different build configurations and their purposes.
41-44: Verify Flutter configuration command
Let's ensure the Flutter configuration command works as expected.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (4)
recipients_app/README.md (4)
7-12: Specify minimum versions for all toolsFor consistency and clarity, please specify the minimum required versions for:
- Android Studio (current: "LadyBug or later")
- VS Code (current: "Latest")
64-66: Add caution note about automatic installationThe automatic
asdf installon terminal start might be unnecessary and time-consuming if tools are already installed. Consider making this step optional or adding a check.
75-76: Add security note about Sentry URL handlingConsider adding a note about secure handling of the Sentry URL, such as:
- Using environment variables
- Not committing the actual URL to version control
80-82: Add verification steps for successful setupConsider adding a verification command to ensure all tools are properly configured, such as:
flutter doctor -v🧰 Tools
🪛 LanguageTool
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and checkflutter --versionis...(OUTSIDE_OF)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipients_app/README.md(4 hunks)recipients_app/clean_build.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
recipients_app/clean_build.sh
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
🪛 LanguageTool
recipients_app/README.md
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~151-~151: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (3)
recipients_app/clean_build.sh (1)
6-10: LGTM!
Good implementation of the Flutter installation check with proper error handling.
recipients_app/README.md (2)
24-26: Use proper placeholders and quotes in environment variables
The environment variable setup needs improvements:
- Replace "YOUR PATH TO YOUR ANDROID SDK" with a more specific placeholder like "$HOME/Library/Android/sdk"
- Add quotes around path variables to handle spaces
-export ANDROID_HOME="YOUR PATH TO YOUR ANDROID SDK"
+export ANDROID_HOME="$HOME/Library/Android/sdk"
-export PATH="$PATH:$ANDROID_HOME/emulator:$ANDROID_HOME/tools:$ANDROID_HOME/tools/bin:$ANDROID_HOME/platform-tools"
+export PATH="$PATH:$ANDROID_HOME/emulator:$ANDROID_HOME/tools:$ANDROID_HOME/tools/bin:$ANDROID_HOME/platform-tools"141-154: LGTM! Comprehensive upgrade instructions
The Flutter version upgrade instructions are clear and cover all necessary files that need to be updated.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~151-~151: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (6)
recipients_app/clean_build.sh (2)
13-20: Consider using semver comparisonThe current version check might fail with pre-release versions or patches.
-if [ "$CURRENT_VERSION" != "$REQUIRED_VERSION" ]; then +if ! command -v semver &> /dev/null; then + echo "Installing semver..." + npm install -g semver +fi +if ! semver -r ">=$REQUIRED_VERSION" "$CURRENT_VERSION" &> /dev/null; then
22-34: Add build artifacts cleanupConsider cleaning additional build artifacts for a thorough cleanup.
echo "Cleaning Flutter build..." flutter clean + +# Clean additional build artifacts +rm -rf .dart_tool/ +rm -rf build/ +rm -rf .packagesrecipients_app/README.md (4)
5-13: Add minimum system requirementsPlease add minimum system requirements for Apple Silicon Mac, such as:
- macOS version
- RAM
- Storage space needed for development tools
24-27: Improve environment variable setup instructionsConsider using a more specific placeholder and add verification steps:
-export ANDROID_HOME="YOUR PATH TO YOUR ANDROID SDK" +export ANDROID_HOME="$HOME/Library/Android/sdk" # Default Android Studio SDK path on macOS export ANDROID_SDK_ROOT="$ANDROID_HOME" export PATH="$PATH:$ANDROID_HOME/emulator:$ANDROID_HOME/tools:$ANDROID_HOME/tools/bin:$ANDROID_HOME/platform-tools" + +# Verify setup with: +echo $ANDROID_HOME +adb version # Should show Android Debug Bridge version
64-66: Add performance warning for automatic installationThe automatic
asdf installon terminal start could impact terminal startup time. Consider adding a warning comment and making it optional:-# This is optional. It installs tools defined in .tool-versions on terminal start -asdf install +# WARNING: Automatic installation on terminal start may slow down terminal startup +# Uncomment the following line to enable automatic installation (optional): +# asdf install
141-156: Add version verification stepsAdd commands to verify the successful version upgrade:
- README.md - Find all places in the README.md which mentions the Flutter version number + +After updating, verify the changes: +```shell +flutter --version # Should show 3.19.6 +dart --version # Should show 3.3.4 +flutter doctor # Should show no issues +```🧰 Tools
🪛 LanguageTool
[uncategorized] ~151-~151: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
recipients_app/README.md(4 hunks)recipients_app/clean_build.sh(1 hunks)
🧰 Additional context used
🪛 Shellcheck (0.10.0)
recipients_app/clean_build.sh
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
🪛 LanguageTool
recipients_app/README.md
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[style] ~80-~80: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~151-~151: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (3)
recipients_app/clean_build.sh (2)
6-11: LGTM: Proper error handling for Flutter installation
The installation check is well-implemented with clear error messaging.
1-4:
Make the script executable
The script needs executable permissions to run properly.
Run:
chmod +x recipients_app/clean_build.sh🧰 Tools
🪛 Shellcheck (0.10.0)
[error] 1-1: ShellCheck only supports sh/bash/dash/ksh/'busybox sh' scripts. Sorry!
(SC1071)
recipients_app/README.md (1)
93-101: Add Firebase emulator setup instructions
For the dev flavor, please add:
- Instructions for setting up Firebase emulators
- Commands to start the emulators
- Common troubleshooting steps
…ocialincome-san/public into karin/add-docu-and-launch-json
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
recipients_app/README.md (1)
5-13: Add system requirementsPlease add:
- Minimum disk space requirements
- Optional tools (e.g., Firebase CLI for local development)
- Links to download pages for each tool
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipients_app/README.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
recipients_app/README.md
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~76-~76: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rl, no issues will be reported to Sentry but you can still run the app. To get t...
(COMMA_COMPOUND_SENTENCE)
[style] ~81-~81: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~152-~152: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (2)
recipients_app/README.md (2)
24-26: Replace hardcoded path with better placeholder
Replace "YOUR PATH TO YOUR ANDROID SDK" with a more specific placeholder:
-export ANDROID_HOME="YOUR PATH TO YOUR ANDROID SDK"
+export ANDROID_HOME="$HOME/Library/Android/sdk" # Default Android SDK location on macOS94-102: Add Firebase emulator setup instructions
For the dev flavor, please add:
- Steps to install and configure Firebase emulators
- Commands to start/stop emulators
- Common troubleshooting steps
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
recipients_app/README.md (1)
5-13: Add tool version requirements and verification stepsPlease add:
- Minimum required disk space
- Required memory specifications
- Commands to verify successful tool installation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
recipients_app/README.md(4 hunks)
🧰 Additional context used
🪛 LanguageTool
recipients_app/README.md
[duplication] ~29-~29: Possible typo: you repeated a word
Context: ...anges take effect - Install Java 17 via Homebrew brew install openjdk@17 - Homebrew is telling you to execute a symlink com...
(ENGLISH_WORD_REPEAT_RULE)
[uncategorized] ~32-~32: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ... can find this JDK. Please do this. - Additionally in your USER's HOME directory in the fi...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[uncategorized] ~77-~77: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rl, no issues will be reported to Sentry but you can still run the app. To get t...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~81-~81: Possible missing comma found.
Context: ...vor and backend environment you want to use and change it if necessary. - Open ...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~84-~84: This phrase is redundant. Consider using “inside”.
Context: ...ject folder in vsCode - Open a terminal inside of vsCode and check flutter --version is...
(OUTSIDE_OF)
[uncategorized] ~155-~155: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
🔇 Additional comments (6)
recipients_app/README.md (6)
24-26: Improve environment variable path examples
Replace generic placeholder with more specific example:
-export ANDROID_HOME="YOUR PATH TO YOUR ANDROID SDK"
+export ANDROID_HOME="$HOME/Library/Android/sdk"55-67: Add verification steps for asdf setup
Add commands to verify successful installation:
asdf current flutter # Should show 3.19.6
asdf which flutter # Should show installation path
flutter doctor # Should show all checks passed75-80: Clarify Sentry URL requirements
Please specify:
- Whether Sentry URL is mandatory for local development
- How to obtain access to Sentry if needed
- What functionality is affected without Sentry
🧰 Tools
🪛 LanguageTool
[uncategorized] ~77-~77: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...rl, no issues will be reported to Sentry but you can still run the app. To get t...
(COMMA_COMPOUND_SENTENCE)
87-88: Add missing dependency installation step
Add the following step before running clean_build.sh:
chmod +x clean_build.sh
+# Install dependencies
+flutter pub get
./clean_build.sh97-105: Add Firebase emulator setup instructions
For the dev flavor, please add:
- Instructions for setting up Firebase emulators
- Commands to start the emulators
- Common troubleshooting steps
145-160: Add validation steps after version upgrade
Please add steps to verify the upgrade:
- How to validate the new version works correctly
- Required testing steps
- Common issues and their solutions
🧰 Tools
🪛 LanguageTool
[uncategorized] ~155-~155: A comma may be missing after the conjunctive/linking adverb ‘Otherwise’.
Context: ...x.y.z with the new Flutter version. - Otherwise just update the version number in the f...
(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Summary by CodeRabbit
Release Notes
New Features
Documentation
asdf.Chores