-
Notifications
You must be signed in to change notification settings - Fork 131
Migrate share_plus to v11.1.0 and upgrade AGP and Gradle version #436
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
Conversation
WalkthroughUpdated share_plus to v11.1.0 and migrated two Dart sharing call sites to the new SharePlus API (using ShareParams). Also updated Android Gradle plugin, Gradle wrapper, and minor Android build file syntax changes. No public app API signatures changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/ui/views/projects/components/featured_project_card.dart (1)
107-115: Optional: DRY a small share helper.
Both this widget and ProjectDetailsView construct ShareParams similarly; consider a tiny util like shareText(BuildContext ctx, String text).lib/ui/views/projects/project_details_view.dart (1)
53-61: Tiny safety guard (optional).
If findRenderObject() ever returns null, early-return to avoid a late NPE; behavior on iPad remains the same when attached.void onShareButtonPressed() { - final RenderBox? box = context.findRenderObject() as RenderBox?; - SharePlus.instance.share( + final RenderBox? box = context.findRenderObject() as RenderBox?; + if (box == null) return; + SharePlus.instance.share( ShareParams( text: _getProjectUrl(), sharePositionOrigin: box!.localToGlobal(Offset.zero) & box.size, ), ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
lib/ui/views/projects/components/featured_project_card.dart(1 hunks)lib/ui/views/projects/project_details_view.dart(1 hunks)pubspec.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test & Analyze (ubuntu-latest)
- GitHub Check: Test & Analyze (macos-latest)
- GitHub Check: Test & Analyze (windows-latest)
🔇 Additional comments (2)
pubspec.yaml (1)
54-54: Bump looks correct and aligns with the new SharePlus API.
No issues with the dependency update itself.lib/ui/views/projects/project_details_view.dart (1)
56-61: LGTM: correct migration to SharePlus with ShareParams and origin Rect.
Matches the v11 API and satisfies iPad’s origin requirement. (pub.dev)
Pull Request Test Coverage Report for Build 17701312068Details
💛 - Coveralls |
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/ui/views/projects/components/featured_project_card.dart (2)
111-117: Guard against a null RenderBox to avoid a rare crash.
box!can be null in edge cases (e.g., if the context has no render object yet). Provide a safe fallbackRect.Apply:
- final box = context.findRenderObject() as RenderBox?; - await SharePlus.instance.share( - ShareParams( - text: projectUrl, - sharePositionOrigin: box!.localToGlobal(Offset.zero) & box.size, - ), - ); + final box = context.findRenderObject() as RenderBox?; + final origin = (box != null) + ? (box.localToGlobal(Offset.zero) & box.size) + : const Rect.fromLTWH(0, 0, 1, 1); + await SharePlus.instance.share( + ShareParams( + text: projectUrl, + sharePositionOrigin: origin, + ), + );
111-117: Cover changed lines with a lightweight widget test.Coveralls flagged 0/4 new lines here. Consider a widget test that taps the share button and verifies a fake platform’s
sharewas invoked with a non‑nullsharePositionOrigin. I can sketch the test usingshare_plus_platform_interfacewith a fake platform if helpful.android/settings.gradle (1)
21-23: Confirm Kotlin plugin version intent; align plugin IDs.You’re declaring
org.jetbrains.kotlin.androidat1.9.25. If this replaced a newer 2.x, confirm the downgrade was intentional and compatible with AGP 8.7 and your Flutter toolchain. Also consider using the same plugin ID inapp/build.gradle(org.jetbrains.kotlin.androidinstead ofkotlin-android) for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
android/app/build.gradle(2 hunks)android/gradle/wrapper/gradle-wrapper.properties(1 hunks)android/settings.gradle(1 hunks)lib/ui/views/projects/components/featured_project_card.dart(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Test & Analyze (macos-latest)
- GitHub Check: Test & Analyze (windows-latest)
- GitHub Check: Test & Analyze (ubuntu-latest)
🔇 Additional comments (4)
android/app/build.gradle (2)
54-54: AGP 8 DSL: compileSdk migration looks good.
compileSdk 35is the correct syntax with AGP 8.x.
70-72: AGP 8 DSL: lintOptions → lint is correct.Migration to the
lint {}block is appropriate; rule name is preserved.lib/ui/views/projects/components/featured_project_card.dart (1)
111-117: Migration to SharePlus.instance + ShareParams is correct; iPad origin handled.Nice move to the new API and providing
sharePositionOrigin.android/gradle/wrapper/gradle-wrapper.properties (1)
6-6: Pin Gradle wrapper checksum and regenerate wrapper JAR (manual verification required)Add distributionSha256Sum to android/gradle/wrapper/gradle-wrapper.properties and regenerate gradle-wrapper.jar so it matches Gradle 8.10.2 — the execution environment could not compute the SHA (hash tools missing). Run locally and update the PR:
- Apply diff:
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-all.zip +distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-all.zip +distributionSha256Sum=<fill_with_official_sha256>
- Compute SHA locally:
curl -fsSL https://services.gradle.org/distributions/gradle-8.10.2-all.zip -o /tmp/gradle-8.10.2-all.zip
sha256sum /tmp/gradle-8.10.2-all.zipor: shasum -a 256 /tmp/gradle-8.10.2-all.zip or openssl dgst -sha256 /tmp/gradle-8.10.2-all.zip
- Regenerate wrapper JAR and commit:
./gradlew wrapper --gradle-version 8.10.2 --distribution-type all
git add android/gradle/wrapper/gradle-wrapper.properties android/gradle/wrapper/gradle-wrapper.jar && git commit -m "Pin Gradle 8.10.2 checksum and regenerate wrapper jar"
|
@hardik17771, please review. I have also addressed #423 |
Fixes #426 #423
Describe the changes you have made in this PR -
share_plusdependency from^10.1.4to^11.1.0Share.share()withSharePlus.instance.share()8.7.01.9.25, it is downgraded because the dependencies were compiled with Kotlin2.1.0, but our build system expects Kotlin1.9.0.8.10.2lintOptions→lintandcompileSdkVersion→compileSdkNote: Please check Allow edits from maintainers. if you would like us to assist in the PR.
Summary by CodeRabbit
Refactor
Chores