Skip to content

fix: format according to dart 3.7 #1387

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

Merged
merged 6 commits into from
Apr 22, 2025
Merged

fix: format according to dart 3.7 #1387

merged 6 commits into from
Apr 22, 2025

Conversation

juliansteenbakker
Copy link
Owner

@juliansteenbakker juliansteenbakker commented Apr 11, 2025

Due to the min sdk being 3.7, we should update our formatting accordingly. However, there is a new option comming that will make it possible to opt-out of this new format. @navaronbracke Do you have any opinion on this?

@codecov-commenter
Copy link

codecov-commenter commented Apr 11, 2025

Codecov Report

Attention: Patch coverage is 19.78022% with 73 lines in your changes missing coverage. Please review.

Project coverage is 36.83%. Comparing base (fbc112d) to head (90e329b).
Report is 41 commits behind head on develop.

Files with missing lines Patch % Lines
lib/src/overlay/scan_window_painter.dart 0.00% 18 Missing ⚠️
.../method_channel/mobile_scanner_method_channel.dart 5.88% 16 Missing ⚠️
lib/src/mobile_scanner_controller.dart 38.88% 11 Missing ⚠️
lib/src/objects/barcode.dart 0.00% 8 Missing ⚠️
lib/src/objects/contact_info.dart 0.00% 7 Missing ⚠️
...hod_channel/android_surface_producer_delegate.dart 0.00% 3 Missing ⚠️
lib/src/method_channel/rotated_preview.dart 0.00% 2 Missing ⚠️
lib/src/mobile_scanner.dart 33.33% 2 Missing ⚠️
lib/src/objects/mobile_scanner_state.dart 0.00% 1 Missing ⚠️
lib/src/objects/phone.dart 0.00% 1 Missing ⚠️
... and 4 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1387      +/-   ##
===========================================
+ Coverage    35.92%   36.83%   +0.91%     
===========================================
  Files           40       41       +1     
  Lines          849      885      +36     
===========================================
+ Hits           305      326      +21     
- Misses         544      559      +15     
Flag Coverage Δ
unittests 36.83% <19.78%> (+0.91%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@navaronbracke
Copy link
Collaborator

@juliansteenbakker I'm aware we needed to redo the formatting here (the change for onSurfaceCleanup needed the min Flutter version to go up). Where in dart-lang/sdk was the new opt-out discussed? The new formatter style is nice, but some things still need tweaking on that front I think (I.e. some of the arrow function formatting still needed fixing)

@juliansteenbakker
Copy link
Owner Author

See dart-lang/dart_style@71ac085 for the merge request. We can enable it in the future with:

formatter:
  trailing_commas: preserve

I think the best for now is to wait until this is landed, and apply asap. Otherwise we will have a lot of git diffs for nothing.

@navaronbracke
Copy link
Collaborator

I noticed it too. They announced it on their socials iirc. I agree we should land the formatting fix as soon as possible.

@juliansteenbakker
Copy link
Owner Author

It seems that trailing_commas will not be available for sdk 3.7, but 3.8 or later. Given that we want to release version 7 with sdk 3.7, i think there is no other option than to format it correctly for this sdk version.

Source: https://github.com/dart-lang/dart_style/blob/main/CHANGELOG.md & dart-lang/dart_style#1707

@navaronbracke
Copy link
Collaborator

I'm aware that the formatter option was branch cut for 3.8 only. Let's just format as-is now and revisit this later. (there will be fixes for the new formatter style anyway, so we might need to do a second formatting pass once those fixes are in)

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.

3 participants