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

fix: resolve flutter static analysis issues (beta) #852

Merged
merged 18 commits into from
Jun 6, 2024

Conversation

srieteja
Copy link
Contributor

@srieteja srieteja commented May 27, 2024

Closes #847
- What I did

  • resolve all issues identified by flutter static analysis on beta channel
  • Android:
    1. Upgraded all example apps compileSdkVersion and targetSdkVersion to 34
    2. Set android:exported to true on all example app Manifest.xml (apps do not build without this change)

- How to verify it

  1. All github actions should pass
  2. Manual testing completed

- Description for the changelog

fix: resolve flutter static analysis issues (beta)

@cpswan cpswan marked this pull request as draft May 27, 2024 11:47
@cpswan
Copy link
Member

cpswan commented May 27, 2024

@srieteja I've converted this to draft until you can confirm that testing has happened. Do you need help from @CurtlyCritchlow ?

It's good to see the automated tests all green :)

@srieteja
Copy link
Contributor Author

I would appreciate the help, if @CurtlyCritchlow does have bandwidth available.

@CurtlyCritchlow
Copy link
Contributor

CurtlyCritchlow commented May 27, 2024

@srieteja Let's check off the packages as we finish manually testing.

  • at_backupkey_flutter
  • at_chat_flutter
  • at_common_flutter
  • at_contacts_flutter
  • at_contacts_group_flutter
  • at_events_flutter
  • at_location_flutter
  • at_follows_flutter
  • at_invitation_flutter
  • at_notify_flutter
  • at_onboarding_flutter
  • at_sync_ui_flutter
  • at_theme_flutter

@CurtlyCritchlow CurtlyCritchlow self-assigned this May 28, 2024
@srieteja
Copy link
Contributor Author

srieteja commented Jun 3, 2024

I've tested the example apps of at_chat_flutter, at_contacts_flutter and at_contacts_group_flutter. All of these show only an onboarding_screen. (I was expecting to see chat/contact widgets on the homepage. Actual widgets might be displayed after onboarding)

Issues Observed (on Android)

  1. at_chat_flutter: when I try to onboard/create a new atsign it just gets stuck on a loading screen
  2. at_contacts_flutter, at_contacts_group_flutter: when I try to fetch a new atsign, it shows an error dialogue "Unauthorized" (Possibly a missing API key or an old one).
  3. There are compatibility issues as the current apps are on 'compileSdkVersion 33', but during build, I was prompted to upgrade to 'compileSdkVersion 34'. Without this change, the build process fails (Not sure if this was something related to my Android SDK setup or current Android platform requirement)

Note: Since these are onboarding issues, the root cause might be lying somewhere in at_onboarding_flutter

Since this PR only contains changes to display elements, likely, the changes in this PR do not cause these issues. But we need a separate ticket to address these issues (@CurtlyCritchlow and I will discuss and create a new ticket for this)

@srieteja
Copy link
Contributor Author

srieteja commented Jun 3, 2024

showcaseview has released a new major version '3.0.0' during the weekend. I will go ahead and upgrade this dependency for all the packages

@CurtlyCritchlow
Copy link
Contributor

CurtlyCritchlow commented Jun 3, 2024

showcaseview has released a new major version '3.0.0' during the weekend. I will go ahead and upgrade this dependency for all the packages

@srieteja, You have to update it in at_backup_flutter. Once that package is updated and published, the issue will be resolved in all the other packages.

@srieteja
Copy link
Contributor Author

srieteja commented Jun 3, 2024

showcaseview has released a new major version '3.0.0' during the weekend. I will go ahead and upgrade this dependency for all the packages

@srieteja, You have to update it in at_backup_flutter. Once that package is updated and published, the issue will be resolved in all the other packages.

Yes @CurtlyCritchlow. I have upgraded it in at_backup_flutter, I'm testing all the other packages using a dep_override for at_backup_flutter

@srieteja
Copy link
Contributor Author

srieteja commented Jun 4, 2024

I've tested the example apps of at_chat_flutter, at_contacts_flutter and at_contacts_group_flutter. All of these show only an onboarding_screen. (I was expecting to see chat/contact widgets on the homepage. Actual widgets might be displayed after onboarding)

Issues Observed (on Android)

  1. at_chat_flutter: when I try to onboard/create a new atsign it just gets stuck on a loading screen
  2. at_contacts_flutter, at_contacts_group_flutter: when I try to fetch a new atsign, it shows an error dialogue "Unauthorized" (Possibly a missing API key or an old one).
  3. There are compatibility issues as the current apps are on 'compileSdkVersion 33', but during the build, I was prompted to upgrade to 'compileSdkVersion 34'. Without this change, the build process fails (Not sure if this was something related to my Android SDK setup or current Android platform requirement)

Note: Since these are onboarding issues, the root cause might be lying somewhere in at_onboarding_flutter

Since this PR only contains changes to display elements, likely, the changes in this PR do not cause these issues. But we need a separate ticket to address these issues (@CurtlyCritchlow and I will discuss and create a new ticket for this)

Update:

  • I was able to run at_chat_flutter, at_contacts_flutter and at_contacts_group_flutter successfully, when using an existing atsign. The problem I faced earlier was because I was trying to create a new atsign through the example widget.

  • Now I can certainly say that there is an issue with at_onboarding_flutter while trying to create a new atsign (will attach the logs at the end)

  • reg the issue mentioned with compileSdkVersion: It's a platform-related issue and from the articles/tickets I've read it's not a serious issue. This is caused if any of our dependencies have migrated to compileSdkVersion 34 and we have not. 'It's a standard practice to update/increase the 'compileSdkVersion' occasionally'
    Ref: Android plugin SDK version warning sounds severe even in cases where it is likely harmless flutter/flutter#143605

  • [Resolved] Faced a new issue with at_location_flutter: I could successfully auth with my atKeys and see the at_location UI, but then the app threw an error-dialog saying 'You are not authorized'. I'm not sure what's causing this, but I'm currently working on looking into this. [EDIT] This error was due to the MAP-APIs API key not being set in the example app.

Problematic Logs while creating a new atsign using at_onboarding_flutter:

will be updated

@CurtlyCritchlow
Copy link
Contributor

@srieteja all the apps work for me. It's a green light for me.

@CurtlyCritchlow CurtlyCritchlow marked this pull request as ready for review June 5, 2024 14:10
@srieteja srieteja self-assigned this Jun 6, 2024
@srieteja srieteja requested a review from cpswan June 6, 2024 07:45
@cpswan
Copy link
Member

cpswan commented Jun 6, 2024

Great work @srieteja and @CurtlyCritchlow

There's a lot going on here, and I think it's 99% good to go. Just one niggle about a(n overlooked?) dependency override.

@srieteja srieteja requested a review from cpswan June 6, 2024 10:07
@srieteja srieteja merged commit 682cd5f into trunk Jun 6, 2024
31 checks passed
@srieteja srieteja deleted the static_analysis_beta_fixes branch June 6, 2024 10:07
@cpswan
Copy link
Member

cpswan commented Jun 6, 2024

@srieteja are you going to take care of publishing these to pub.dev now?

@srieteja
Copy link
Contributor Author

srieteja commented Jun 6, 2024

@cpswan, I will publish at_backupkey_flutter and at_onboarding_flutter later today, as these are key packages. @CurtlyCritchlow and I can coordinate on publishing the rest.

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.

Clear up info messages from flutter analyze
3 participants