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

feat: add flutter_lints, fix or ignore lints #414

Merged
merged 10 commits into from
May 18, 2024

Conversation

josxha
Copy link
Collaborator

@josxha josxha commented May 17, 2024

Still a couple hundered changed lines but I hope that it's now better reviewable. If not, I separated the changes into different commits.
See the commit history for more details on the changes.

@josxha josxha changed the title feat: add flutter_lints, fix or ignore lints feat: add flutter_lints, fix or ignore lints May 17, 2024
@kuhnroyal
Copy link
Collaborator

I am not sure if all the changes are supported on the minimum SDK, (context.mounted for example?).

I created #416 to ensure that we always build the min SDK and stable.
If you want, you can cherry pick that and fix the initial lints. Then I can merge it. And afterwards you can add flutter_lint and see if it builds.

Or you can pick those changes into this branch.

@josxha
Copy link
Collaborator Author

josxha commented May 17, 2024

I am not sure if all the changes are supported on the minimum SDK, (context.mounted for example?).

It got added 2 years ago, so I'm about certain it is supported. flutter/flutter#111619
Is an outdated flutter version really something we need to be that careful about at the moment?

If you want, you can cherry pick that and fix the initial lints. Then I can merge it. And afterwards you can add flutter_lint and see if it builds.

Fixing all those doc strings took too much time so that I'd rather do it not again. I cherry picked your updated CI into a new branch that is based on this. https://github.com/josxha/flutter-maplibre-gl/actions/runs/9132300475

Resolving dependencies...
The current Dart SDK version is 3.0.5.

Because maplibre_gl_example depends on flutter_lints >=3.0.2 which requires SDK version >=3.1.0 <4.0.0, version solving failed.
The Flutter CLI developer tool uses Google Analytics to report usage and diagnostic data
along with package dependencies, and crash reporting to send basic crash reports.
This data is used to help improve the Dart platform, Flutter framework, and related tools.

flutter_lints version 4 requires at least dart version 3.2.0.

https://github.com/flutter/packages/blob/ae4dd32f4a661cd0d46b4541445d0222856b5173/packages/flutter_lints/pubspec.yaml#L7-L8

@josxha
Copy link
Collaborator Author

josxha commented May 17, 2024

After I bumped the flutter version to 3.16.0, all jobs succeed.
https://github.com/josxha/flutter-maplibre-gl/actions/runs/9132493036/job/25113958221

@kuhnroyal
Copy link
Collaborator

It got added 2 years ago, so I'm about certain it is supported. flutter/flutter#111619

Nice, so this is fine.

flutter_lints version 4 requires at least dart version 3.2.0.

Lets just use ^3.0.0 for now so we can stick with Flutter 3.10.0

@josxha
Copy link
Collaborator Author

josxha commented May 17, 2024

Lets just use ^3.0.0 for now so we can stick with Flutter 3.10.0

I used flutter_lints: '>=3.0.0 <5.0.0' but because of the textScaler parameter has changed the CI fails.

Compiling lib/main.dart for the Web...                          
Target dart2js failed: Exception: lib/sources.dart:352:3[9](https://github.com/josxha/flutter-maplibre-gl/actions/runs/9134655373/job/25120633165#step:5:10):
Error: Couldn't find constructor 'TextScaler.linear'.
                    textScaler: const TextScaler.linear(1.4),
                                      ^^^^^^
lib/sources.dart:352:21:
Error: No named parameter with the name 'textScaler'.
                    textScaler: const TextScaler.linear(1.4),
                    ^^^^^^^^^^
/opt/hostedtoolcache/flutter/stable-3.[10](https://github.com/josxha/flutter-maplibre-gl/actions/runs/9134655373/job/25120633165#step:5:11).5-x64/packages/flutter/lib/src/widgets/text.dart:428:9:
Info: Found this candidate, but the arguments don't match.
  const Text(
        ^^^^
Error: Compilation failed.

josxha and others added 5 commits May 18, 2024 00:02
* ensure examples are being built on minimum supported SDK and latest stable SDK
* remove automatic formatter workflow, we ensure correct formatting on every PR and since formatting is Dart version dependent, this might not always work
* remove formatting/analyze steps from beta CI - same as above formatting and analysis should always use latest stable as baseline
@kuhnroyal
Copy link
Collaborator

I picked the CI update into this branch and replaced the TextScaler/textScale usage in the example with a simple textStyle. Now we can use flutter_lints: ^3.0.0 and support Flutter 3.10.0.

@kuhnroyal kuhnroyal merged commit 8323f55 into maplibre:main May 18, 2024
9 checks passed
@kuhnroyal kuhnroyal mentioned this pull request May 18, 2024
@josxha josxha added this to the v0.19.0 milestone May 20, 2024
@josxha josxha self-assigned this May 20, 2024
josxha added a commit to josxha/flutter-maplibre-gl that referenced this pull request May 21, 2024
commit f27bcab
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Sat May 18 23:49:28 2024 +0200

    feat: allow latest `flutter_lints` version (maplibre#419)

commit 8323f55
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Sat May 18 02:00:56 2024 +0200

    feat: add `flutter_lints`, fix or ignore lints (maplibre#414)

    Co-authored-by: Peter Leibiger <kuhnroyal@gmail.com>

commit e5d95ed
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Fri May 17 23:38:40 2024 +0200

    fix(web): ensure the usage of `maplibre-gl-js` version 4.x.x, remove `_addStylesheetToShadowRoot` (maplibre#409)

commit e617b90
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Fri May 17 16:33:47 2024 +0200

    feat: update package links in pubspec.yaml files (maplibre#413)

commit 68040d4
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Fri May 17 16:25:36 2024 +0200

    chore: delete `pubspec.lock` files (maplibre#412)

commit 3db5b2a
Author: Fabian Keunecke <github@fabiankeunecke.de>
Date:   Fri May 17 14:58:33 2024 +0200

    Support newer Gradle versions (maplibre#390)

commit 7cb6521
Author: Joscha <34318751+josxha@users.noreply.github.com>
Date:   Fri May 17 14:53:47 2024 +0200

    feat(web): allow package:js version 0.6.x and 0.7.x (maplibre#410)
@josxha josxha deleted the clean/part3-2 branch June 3, 2024 10:30
Remi-deronzier pushed a commit to viamichelin/flutter-maplibre-gl that referenced this pull request Sep 6, 2024
Co-authored-by: Peter Leibiger <kuhnroyal@gmail.com>
Remi-deronzier pushed a commit to viamichelin/flutter-maplibre-gl that referenced this pull request Sep 10, 2024
Co-authored-by: Peter Leibiger <kuhnroyal@gmail.com>
Remi-deronzier pushed a commit to viamichelin/flutter-maplibre-gl that referenced this pull request Sep 10, 2024
Co-authored-by: Peter Leibiger <kuhnroyal@gmail.com>
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.

2 participants