chore(recipients-app): Bump android versions and dependencies #959
Conversation
MDikkii
requested review from
andrashee,
mkue,
ssandino and
novas1r1
as code owners
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Skipped Deployment
|
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve multiple updates across various files in the Android and Flutter application. Key modifications include the addition of a namespace and application ID in the Changes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (2)recipients_app/android/app/build.gradle (2)
The namespace matches the applicationId, following Android best practices.
The Java 17 and matching Kotlin JVM target are correctly configured, which is required for newer Android Gradle Plugin versions. Let's verify the Gradle and plugin versions are compatible with Java 17: ✅ Verification successfulJava/Kotlin configuration is compatible with toolchain versions The setup is correct and compatible:
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check Gradle wrapper version
cat recipients_app/android/gradle/wrapper/gradle-wrapper.properties | grep 'distributionUrl'
# Check plugin versions in settings.gradle
cat recipients_app/android/settings.gradle | grep -A 1 'com.android.application\|org.jetbrains.kotlin.android'
Length of output: 519 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: 2
🧹 Outside diff range and nitpick comments (1)
recipients_app/android/settings.gradle (1)
21-22: Document upgrade steps for the teamThese are significant version jumps that may require additional configuration changes:
- AGP 7.x → 8.x
- Kotlin 1.7 → 1.9
Consider adding upgrade notes in the PR description or README about:
- Required Gradle version
- Required Java version
- Breaking changes and migration steps
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
recipients_app/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (8)
recipients_app/android/app/build.gradle(2 hunks)recipients_app/android/gradle/wrapper/gradle-wrapper.properties(1 hunks)recipients_app/android/settings.gradle(1 hunks)recipients_app/lib/kri_intl.dart(4 hunks)recipients_app/lib/ui/buttons/button_outlined_big.dart(1 hunks)recipients_app/lib/ui/buttons/button_small.dart(2 hunks)recipients_app/pubspec.yaml(1 hunks)recipients_app/test/helpers/golden_test_device_scenario.dart(0 hunks)
💤 Files with no reviewable changes (1)
- recipients_app/test/helpers/golden_test_device_scenario.dart
✅ Files skipped from review due to trivial changes (1)
- recipients_app/android/gradle/wrapper/gradle-wrapper.properties
🔇 Additional comments (8)
recipients_app/lib/ui/buttons/button_outlined_big.dart (1)
24-25: Verify Flutter version compatibility for WidgetStateProperty usage
The switch from MaterialStateProperty to WidgetStateProperty needs version verification.
✅ Verification successful
WidgetStateProperty usage is compatible with Flutter version
The migration from MaterialStateProperty to WidgetStateProperty is appropriate as the project uses Flutter 3.19.6, where this change is supported. The SDK constraints (>=3.2.0) also align with this migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check Flutter version constraints in pubspec.yaml
rg "sdk: flutter" -A 2 "pubspec.yaml"
Length of output: 90
Script:
#!/bin/bash
# Search for pubspec files recursively
fd "pubspec.yaml"
# Check for any MaterialStateProperty to WidgetStateProperty migrations
rg "WidgetStateProperty" -A 2
Length of output: 1604
Script:
#!/bin/bash
# Check Flutter SDK constraints in the pubspec
cat recipients_app/pubspec.yaml | grep -A 5 "environment:"
Length of output: 150
recipients_app/lib/ui/buttons/button_small.dart (1)
34-36: LGTM! Consistent implementation across button components
The WidgetStateProperty implementation is consistent with ButtonOutlinedBig and maintains the same styling behavior.
Also applies to: 59-61
recipients_app/pubspec.yaml (1)
28-28: Verify breaking changes in intl 0.19.0
The upgrade from intl 0.18.0 to 0.19.0 is a minor version bump that may contain breaking changes.
✅ Verification successful
Intl 0.19.0 upgrade appears safe for this codebase
The codebase uses standard date and number formatting patterns that are stable across intl versions. All usage is concentrated in kri_intl.dart with proper locale handling and standard format strings that are compatible with 0.19.0.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential breaking changes and compatibility issues
# Search for intl usage patterns that might be affected
echo "Checking intl usage patterns..."
rg "DateFormat|NumberFormat|Intl\." -t dart
# Look for existing localization files
echo "Checking l10n/intl files..."
fd -e arb
Length of output: 4691
recipients_app/android/app/build.gradle (3)
35-36: Namespace declaration is correctly implemented.
The namespace addition is necessary for compatibility with Android Gradle Plugin 8.x and follows the recommended practice of matching the applicationId.
89-91: Kotlin JVM target correctly matches Java version.
The Kotlin JVM target is properly aligned with the Java compatibility level, ensuring consistent bytecode generation.
84-87: Consider upgrading Java version if possible.
While Java 1.8 is compatible, newer versions are available. Verify if Flutter supports using a more recent Java version for potential performance improvements and newer language features.
recipients_app/lib/kri_intl.dart (2)
416-416: LGTM!
The delegate declarations are properly implemented.
Also applies to: 665-665
461-461: LGTM!
The DatePickerDateTimeOrder property is correctly implemented.
recipients_app/lib/kri_intl.dart
Outdated
|
|
||
| @override | ||
| String get clearButtonTooltip => "Clear text"; | ||
|
|
||
| @override | ||
| // TODO: implement selectedDateLabel | ||
| String get selectedDateLabel => "Selected"; |
There was a problem hiding this comment.
Implement the selectedDateLabel method
The TODO comment indicates incomplete implementation. The current placeholder value "Selected" should be translated to Krio language to maintain consistency with other translations in the file.
Would you like me to help implement the proper Krio translation for this label? I can also create a GitHub issue to track this task.
MDikkii
changed the title
| @@ -27,7 +27,6 @@ class GoldenTestDeviceScenario extends StatelessWidget { | |||
| size: device.size, | |||
| padding: device.safeArea, | |||
| platformBrightness: device.brightness, | |||
| textScaler: TextScaler.linear(device.textScaleFactor), | |||
There was a problem hiding this comment.
Why did you removed this line? Did you run the tests and then it failed and you had to remove this line?
There was a problem hiding this comment.
No, new version of the dependency just show that there is no more TextScaler 🤔
| @@ -25,7 +25,7 @@ dependencies: | |||
| sdk: flutter | |||
| flutter_native_splash: ^2.4.0 | |||
|
|
|||
| intl: ^0.18.0 | |||
There was a problem hiding this comment.
I think you are using the wrong Flutter version. We are still on 3.19.6
So... I tried to run the app with Android Studio Ladybug installed and made the changes until the build worked again.
The one problem with this solution is that
Sentryseems to be crashing at the start. That means that we may lose the sentry logs if it can't initialize properly.Summary by CodeRabbit
Release Notes
New Features
Improvements
intldependency to version 0.19.0 for potential new features and fixes.Bug Fixes
Chores