parse-community / Parse-SDK-Flutter

The Dart/Flutter SDK for Parse Platform
https://parseplatform.org
Apache License 2.0
575 stars 190 forks source link

refactor: Resolve analyzer info errors in Flutter SDK example #909

Closed Nidal-Bakir closed 1 year ago

Nidal-Bakir commented 1 year ago

Pull Request

Issue

Closes: #910

Approach

Tasks

parse-github-assistant[bot] commented 1 year ago

Thanks for opening this pull request!

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: -2.75 :warning:

Comparison is base (e097da5) 41.88% compared to head (4ce07d6) 39.13%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #909 +/- ## ========================================== - Coverage 41.88% 39.13% -2.75% ========================================== Files 51 56 +5 Lines 2994 3278 +284 ========================================== + Hits 1254 1283 +29 - Misses 1740 1995 +255 ``` [see 5 files with indirect coverage changes](https://app.codecov.io/gh/parse-community/Parse-SDK-Flutter/pull/909/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=parse-community)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

mtrezza commented 1 year ago

CI failed, should this PR be merged anyway because it fixes only a part?

mbfakourii commented 1 year ago

CI failed, should this PR be merged anyway because it fixes only a part?

This error is related to Dart version, I think you should merge #908 first, then merge this PR

mtrezza commented 1 year ago

CI is still failing.

mbfakourii commented 1 year ago

We have another warning in Flutter 3.10 info • Use 'const' with the constructor to improve performance • packages/flutter/example/lib/live_list/main.dart:52:23 • prefer_const_constructors

@Nidal-Bakir I think it needs const

Nidal-Bakir commented 1 year ago

Thats a problem! We can not use const here because its only support in flutter 3.10.0

mbfakourii commented 1 year ago

Thats a problem! We can not use const here because its only support in flutter 3.10.0

I think it is better to create a variable and then give the variable to child

Nidal-Bakir commented 1 year ago

That's a good idea. but it is a temporary solution.

I'm starting to smell a revert PR. for this PR #906 and remove the --fatal-infos flag from the CI/CD

Any new rules/lints in Flutter 3.10.0 that can not be applied in Flutter <3.10.0 will cause a problem.

Nidal-Bakir commented 1 year ago

@mbfakourii
I think we need to remove the --fatal-infos flag from CI/CD. because we are testing on Flutter version 3.3 up to 3.10. More issues like this will rise.

mbfakourii commented 1 year ago

@mbfakourii
I think we need to remove the --fatal-infos flag from CI/CD. because we are testing on Flutter version 3.3 up to 3.10. More issues like this will rise.

Are the warnings completely hidden by removing this flag, for example the format code warning?

Nidal-Bakir commented 1 year ago

This has nothing to do with the code format. That's a other command called dart format

mbfakourii commented 1 year ago

This has nothing to do with the code format. That's a other command called dart format

I'm afraid we're missing some warnings, if you're sure, then remove them

Nidal-Bakir commented 1 year ago

We will likely miss some warning or info messages if we remove the --fatal-infos flag. Alternatively, we could monitor if this causes any issues and remove it if necessary.

Nidal-Bakir commented 1 year ago

We are good to go!