Skip to content
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

chore: sample app fixes #136

Merged
merged 3 commits into from
Aug 13, 2024
Merged

chore: sample app fixes #136

merged 3 commits into from
Aug 13, 2024

Conversation

mrehan27
Copy link
Contributor

@mrehan27 mrehan27 commented Aug 9, 2024

part of MBL-420

Changes

  • Fixed sample app so it can compile
  • Updated build.gradle as required by flutter_local_notifications
  • Updated pub dependencies to reduce challenges on CI builds

@mrehan27 mrehan27 self-assigned this Aug 9, 2024
@mrehan27 mrehan27 requested a review from a team August 9, 2024 15:39
@@ -40,6 +43,7 @@ android {
targetSdkVersion 33
versionCode 1
versionName "1.0"
multiDexEnabled true
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this increases build times when it's enabled so just calling it out to make sure this is required and it wasn't automatically added or you forgot to remove it. If you added it intentionally, no problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. This too was added only for flutter_local_notifications gradle setup.


extension LocationExtensions on GoRouter {
// Get location of current route
// This is a workaround to get the current location as location property
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is labeled as a workaround, it makes me wonder if this is a short-term solution? To make our sample app an up-to-date reference for customers, should we replace this with recommended best practice code in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. I'm not expert with GoRouter so not fully sure about best practices for it at the moment. But yes, we can put at a ticket to simplify and improve this for customers. Sample apps project could be the best time to handle this? 🤔

ndkVersion flutter.ndkVersion

compileOptions {
// Flag to enable support for the new language APIs
coreLibraryDesugaringEnabled true
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

interesting, what new language API are we introducing?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Base automatically changed from rehan/mbl-420-gitignore-updates to main August 13, 2024 10:55
@mrehan27 mrehan27 merged commit 106d3dd into main Aug 13, 2024
3 of 4 checks passed
@mrehan27 mrehan27 deleted the rehan/mbl-420-sample-app-fixes branch August 13, 2024 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants