maplibre / flutter-maplibre-gl

Customizable, performant and vendor-free vector and raster maps, flutter wrapper for maplibre-native and maplibre-gl-js (fork of flutter-mapbox-gl/maps)
https://pub.dev/packages/maplibre_gl
Other
186 stars 106 forks source link

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

Closed josxha closed 1 month ago

josxha commented 1 month ago

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.

kuhnroyal commented 1 month ago

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

I created https://github.com/maplibre/flutter-maplibre-gl/pull/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 commented 1 month ago

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. https://github.com/flutter/flutter/pull/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 commented 1 month ago

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 commented 1 month ago

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 commented 1 month ago

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.
kuhnroyal commented 1 month ago

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.