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: Clear Up Info Messages for at_widget packages using flutter 3.23 beta channel #860

Conversation

CurtlyCritchlow
Copy link
Contributor

@CurtlyCritchlow CurtlyCritchlow commented Jun 11, 2024

- What I did
closes #859
- How I did it
Run flutter analyze to see info message.
Make the changes to clear the info message
- How to verify it
Github static analyser should run successfully.
- Description for the changelog

Clear up info messages

@CurtlyCritchlow CurtlyCritchlow self-assigned this Jun 11, 2024
@CurtlyCritchlow CurtlyCritchlow marked this pull request as draft June 11, 2024 13:30
…tions info message in the example app of at_sync_ui_flutter
@CurtlyCritchlow
Copy link
Contributor Author

@cpswan One of the check failed. This because onPopInvokedWithResult is the replacement method on the flutter 3.23 beta for onPopInvoked but onPopInvokedWithResult is not available on the stable channel. I'll have to revert to onPopInvoked but this will be the only info message on the beta channel.

@cpswan
Copy link
Member

cpswan commented Jun 14, 2024

OK @CurtlyCritchlow

Can we put an annotation in place to supress that info message, and an issue to remind us to make the change once it lands in stable.

Breaking changes > Generic types in PopScope for reference.

CurtlyCritchlow and others added 2 commits June 14, 2024 08:19
… • 'onPopInvoked' is deprecated and shouldn't be used. Use onPopInvokedWithResult instead.
cpswan
cpswan previously approved these changes Jun 14, 2024
Copy link
Member

@cpswan cpswan left a comment

Choose a reason for hiding this comment

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

This may be premature, as the PR is still draft, but I'm happy for this to be merged, so long as your confident with tests.

The one thing I did notice, which can be changed now, or later, or maybe even left, is the example pubspecs still use a very old flutter_lints

@@ -41,7 +41,7 @@ dependencies:
# activated in the `analysis_options.yaml` file located at the root of your
# package. See that file for information about deactivating specific lint
# rules and activating additional ones.
flutter_lints: ^1.0.0
flutter_lints: ^2.0.2
Copy link
Member

Choose a reason for hiding this comment

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

Why not 4.0.0 here?

@@ -42,6 +43,8 @@ dependencies:
cupertino_icons: ^1.0.6
flutter:
sdk: flutter
flutter_dotenv: ^5.1.0
path_provider: ^2.1.3

dev_dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

Very old flutter_lints here too.

path_provider: ^2.1.3
at_common_flutter: ^2.0.13
at_lookup: ^3.0.47
flutter_dotenv: ^5.1.0

dev_dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

and here

@@ -37,6 +39,7 @@ dependencies:
cupertino_icons: ^1.0.6
flutter:
sdk: flutter
path_provider: ^2.1.3

dev_dependencies:
Copy link
Member

Choose a reason for hiding this comment

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

and here

# The following adds the Cupertino Icons font to your application.
# Use with the CupertinoIcons class for iOS style icons.
cupertino_icons: ^1.0.6
flutter:
sdk: flutter
flutter_colorpicker: ^1.1.0
path_provider: ^2.1.3

Copy link
Member

Choose a reason for hiding this comment

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

and here

at_common_flutter: ^2.0.12
at_onboarding_flutter: ^6.1.7
at_theme_flutter:
path: ../
cupertino_icons: ^1.0.6
flutter:
sdk: flutter
path_provider: ^2.1.3

Copy link
Member

Choose a reason for hiding this comment

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

and here

@CurtlyCritchlow
Copy link
Contributor Author

@cpswan I'm about to inbox you about a strategy for publishing the new versions of our packages. I didn't update the dependencies as yet because in some cases it cause conflicts due to other at_widgets packages that are dependencies using older versions and not the newer versions.

@CurtlyCritchlow CurtlyCritchlow requested a review from cpswan June 17, 2024 13:17
@CurtlyCritchlow CurtlyCritchlow marked this pull request as ready for review June 17, 2024 13:17
@CurtlyCritchlow CurtlyCritchlow merged commit a1545b6 into trunk Jun 17, 2024
31 checks passed
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 (Flutter beta 3.23 edition)
2 participants