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

update: support flutter 3.16 dart 3.2 #59

Merged
merged 6 commits into from
Nov 17, 2023

Conversation

ronnnnn
Copy link
Owner

@ronnnnn ronnnnn commented Nov 17, 2023

Overview

Support Flutter 3.16.0 and Dart 3.2.

Feature type

  • Lint Rule
  • Quick fix
  • Assist
  • Other (Update docs, Configure CI, etc...)

@ronnnnn ronnnnn added feat New feature or request dep Dependency updates or problems labels Nov 17, 2023
Copy link
Contributor

github-actions bot commented Nov 17, 2023

Image description CodeRabbit

Walkthrough

The changes primarily focus on updating the codebase and test suite to support Flutter 3.16.0 and Dart 3.2. This includes version updates, lint rule modifications, introduction of new priority constants, and logic adjustments in classes and methods. Deprecated members are used in tests, which may require future attention.

Changes

Files Summary
.tool-versions, packages/nilts/README.md Updated Flutter and Dart versions to 3.16.0 and 3.2 respectively.
packages/nilts/lib/src/lints/fixed_text_scale_factor_rich_text.dart, packages/nilts/lib/src/lints/fixed_text_scale_rich_text.dart Replaced old lint rule FixedTextScaleFactorRichText with a new one FixedTextScaleRichText.
packages/nilts/lib/src/change_priority.dart Added a new priority constant _AddTextScaler to the ChangePriority class.
packages/nilts/lib/src/lints/unnecessary_rebuilds_from_media_query.dart Updated UnnecessaryRebuildsFromMediaQuery class with a new constructor parameter _dartVersion and modified logic in the visitMethodInvocation method.
packages/nilts_test/test/lints/unnecessary_rebuilds_from_media_query.dart Introduced use of deprecated member textScaler from MediaQuery class and added new variables onOffSwitchLabels and maybeOnOffSwitchLabels from MediaQuery class for testing.

Uplevel your code reviews with CodeRabbit Pro

CodeRabbit Pro

If you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Image description CodeRabbit

Commits Files that changed from the base of the PR and between a45d0df and 7cfae94 commits.
Files selected (9)
  • .tool-versions (1)
  • packages/nilts/README.md (3)
  • packages/nilts/lib/nilts.dart (2)
  • packages/nilts/lib/src/change_priority.dart (1)
  • packages/nilts/lib/src/lints/fixed_text_scale_factor_rich_text.dart (1)
  • packages/nilts/lib/src/lints/fixed_text_scale_rich_text.dart (1)
  • packages/nilts/lib/src/lints/unnecessary_rebuilds_from_media_query.dart (3)
  • packages/nilts_test/test/lints/fixed_text_scale_rich_text.dart (2)
  • packages/nilts_test/test/lints/unnecessary_rebuilds_from_media_query.dart (5)
Files not summarized due to errors (1)
  • packages/nilts_test/test/lints/fixed_text_scale_rich_text.dart (nothing obtained from openai)
Files not reviewed due to errors (4)
  • packages/nilts_test/test/lints/unnecessary_rebuilds_from_media_query.dart (no response)
  • packages/nilts/README.md (no response)
  • packages/nilts/lib/src/lints/fixed_text_scale_rich_text.dart (no response)
  • packages/nilts/lib/src/lints/unnecessary_rebuilds_from_media_query.dart (no response)
Files skipped from review due to trivial changes (3)
  • .tool-versions
  • packages/nilts/lib/src/change_priority.dart
  • packages/nilts/lib/src/lints/fixed_text_scale_factor_rich_text.dart
Review comments generated (4)
  • Review: 3
  • LGTM: 1

Tips

Chat with Image description CodeRabbit Bot (@coderabbitai)

  • Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
  • Invite the bot into a review comment chain by tagging @coderabbitai in a reply.

Code suggestions

  • The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
  • You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.

Pausing incremental reviews

  • Add @coderabbitai: ignore anywhere in the PR description to pause further reviews from the bot.

@@ -1,3 +1,5 @@
// ignore_for_file: deprecated_member_use
Copy link
Contributor

Choose a reason for hiding this comment

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

Image description CodeRabbit

The addition of // ignore_for_file: deprecated_member_use is a good move as it suppresses warnings about the use of deprecated members in this file. However, it's important to note that ignoring these warnings should be temporary and the deprecated members should be replaced with their newer counterparts as soon as possible.

+ // ignore_for_file: deprecated_member_use

Comment on lines +18 to +28
// expect_lint: fixed_text_scale_rich_text
RichText(
text: const TextSpan(
text: 'Hello World!',
),
),
RichText(
text: const TextSpan(
text: 'Hello World!',
),
textScaler: MediaQuery.textScalerOf(context),
Copy link
Contributor

Choose a reason for hiding this comment

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

Image description CodeRabbit

The change from fixed_text_scale_factor_rich_text to fixed_text_scale_rich_text is consistent with the updates mentioned in the PR description. However, the usage of textScaler which is a deprecated member of MediaQuery class is not recommended. It would be better to replace it with its updated counterpart if available.

- // expect_lint: fixed_text_scale_factor_rich_text
+ // expect_lint: fixed_text_scale_rich_text
...
- textScaler: MediaQuery.textScalerOf(context),

Comment on lines +15 to +27
final _dartVersion = DartVersion.fromPlatform();

@override
List<LintRule> getLintRules(CustomLintConfigs configs) => [
const DefinedVoidCallbackType(),
const FixedTextScaleFactorRichText(),
if (_dartVersion >= const DartVersion(major: 3, minor: 2, patch: 0))
const FixedTextScaleRichText()
else
const FixedTextScaleRichTextLegacy(),
const FlakyTestsWithSetUpAll(),
const ShrinkWrappedScrollView(),
const UnnecessaryRebuildsFromMediaQuery(),
if (_dartVersion >= const DartVersion(major: 3, minor: 0, patch: 0))
UnnecessaryRebuildsFromMediaQuery(_dartVersion),
Copy link
Contributor

Choose a reason for hiding this comment

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

Image description CodeRabbit

The changes made here are good, but there's a potential issue. The DartVersion comparison in the conditional statements (lines 20 and 26) might not work as expected if the Dart version has a pre-release suffix like 3.2.0-beta.1. It would be better to use a method that compares only the major, minor, and patch versions without considering the pre-release suffix.

- if (_dartVersion >= const DartVersion(major: 3, minor: 2, patch: 0))
+ if (_dartVersion.isAtLeast(const DartVersion(major: 3, minor: 2, patch: 0)))

- if (_dartVersion >= const DartVersion(major: 3, minor: 0, patch: 0))
+ if (_dartVersion.isAtLeast(const DartVersion(major: 3, minor: 0, patch: 0)))

In the DartVersion class, you can add a new method isAtLeast:

bool isAtLeast(DartVersion other) {
  return this.major > other.major ||
      (this.major == other.major && this.minor > other.minor) ||
      (this.major == other.major && this.minor == other.minor && this.patch >= other.patch);
}

This will ensure that the comparison works correctly even when the Dart version has a pre-release suffix.

@ronnnnn ronnnnn merged commit f2294fc into main Nov 17, 2023
4 checks passed
@ronnnnn ronnnnn deleted the update-support-flutter-3.16-dart-3.2 branch November 17, 2023 02:20
@github-actions github-actions bot mentioned this pull request Sep 6, 2024
@github-actions github-actions bot mentioned this pull request Nov 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dep Dependency updates or problems feat New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant