chore(deps): remove old dart code linter reference and bump dcm/lints #269
Conversation
📝 WalkthroughWalkthroughThis pull request encompasses a comprehensive set of updates across multiple packages and configuration files. The changes primarily focus on updating dependency versions, refining code analysis configurations, and making minor improvements to code clarity and documentation. Key areas of modification include SDK and package version upgrades, adjustments to linting and metrics rules, and subtle refinements in code structure across various components of the project. Changes
Sequence DiagramSince the changes are primarily configuration and dependency updates, a sequence diagram would not provide meaningful insights into the modifications. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (3)
tools/lib/utils/json_utils.dart (1)
Line range hint
267-278: Consider clarifying the logic for assigningcountryCodeandscriptCode.This segment uses nested ternary operations and variable reassignments that can be confusing and potentially error-prone. For instance,
countryCodeis first set tomatch?.group(3), thenscriptCodeis conditionally set based on that value, and finally,countryCodeis reassigned based onscriptCode. This makes the logic difficult to follow and creates a risk of mixing up “country” vs. “script” when reading or maintaining the code.You can simplify by clearly separating and naming steps for extracting and reassigning these values, rather than reusing the same variable names for different purposes. For improved readability, consider an approach similar to:
({String? countryCode, String languageCode, String? scriptCode}) _extractLocaleCode(String code) { final regex = RegExp("^([A-Z]+)(?:_([A-Z]+))?(?:_([A-Z]+))?"); final match = regex.firstMatch(code.toUpperCase()); final lang = match?.group(1) ?? ""; - final country = match?.group(2); - String? countryCode = match?.group(3); - String? scriptCode = countryCode == null ? null : country; - countryCode = scriptCode == null ? country : countryCode; + final extractedCountry = match?.group(2); + final extractedScriptOrCountry = match?.group(3); + String? resolvedScriptCode; + String? resolvedCountryCode; + if (extractedScriptOrCountry == null) { + resolvedScriptCode = null; + resolvedCountryCode = extractedCountry; + } else { + // e.g. "en_US_Latn" => extractedCountry == "US", extractedScriptOrCountry == "LATN" + resolvedScriptCode = extractedScriptOrCountry.length == 4 ? extractedScriptOrCountry : extractedCountry; + resolvedCountryCode = extractedScriptOrCountry.length == 4 ? null : extractedScriptOrCountry; + } // Ensure countryCode is not empty - countryCode = (countryCode?.isEmpty ?? true) ? null : countryCode; - if (countryCode?.length == 4) { - scriptCode = countryCode; - countryCode = null; - } + resolvedCountryCode = (resolvedCountryCode?.isEmpty ?? true) ? null : resolvedCountryCode; return ( languageCode: lang, - countryCode: countryCode, - scriptCode: scriptCode, + countryCode: resolvedCountryCode, + scriptCode: resolvedScriptCode, ); }packages/world_countries/pubspec.yaml (1)
SDK constraints need to be lowered to match other packages
The package's SDK constraints (Dart SDK ^3.3.0 and Flutter >=3.19.0) are higher than necessary based on the analysis of other packages in the repository:
- Most packages use either:
- Dart SDK ^3.0.0 (sealed_countries, sealed_currencies, sealed_languages, world_flags)
- Dart SDK ^3.6.0 (tools, examples)
- The world_flags package, which is similar in nature, uses Flutter ">=3.10.0"
Recommendation: Consider lowering the constraints to match other packages:
- Dart SDK: ^3.0.0 (to align with other main packages)
- Flutter: ">=3.10.0" (to match world_flags package)
This would maintain consistency across the project and avoid unnecessarily restricting users to newer versions when the functionality likely works with older versions.
🔗 Analysis chain
Line range hint
36-37: Verify SDK and Flutter version constraints.The package requires Dart SDK ^3.3.0 and Flutter >=3.19.0, which are relatively high version constraints. Please ensure these versions align with the project's minimum supported versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Flutter/Dart SDK constraints across the project echo "Checking SDK constraints across all packages..." fd -e yaml pubspec.yaml | xargs awk '/sdk:/ || /flutter:/' 2>/dev/null echo "Checking Flutter version in pubspec.lock files..." fd -e lock pubspec.lock | xargs grep -A 1 '"flutter"' 2>/dev/nullLength of output: 779
Script:
#!/bin/bash # Let's get a clearer view of SDK constraints by examining each pubspec.yaml individually echo "=== Examining individual pubspec.yaml files ===" for file in $(fd -e yaml pubspec.yaml); do echo -e "\nFile: $file" echo "----------------------------------------" cat "$file" | grep -A 2 "environment:" || true echo "----------------------------------------" doneLength of output: 3971
packages/world_countries/lib/src/widgets/pickers/basic_picker_state.dart (1)
Line range hint
46-57: Consider simplifying the nested conditional expression.While the theme fallback implementation is correct, the nested conditional expression could be simplified for better readability.
Consider extracting the header logic to a separate method or using a more declarative approach:
- header: (widget.showHeader ?? theme?.showHeader ?? true) - ? (widget.header ?? - theme?.header ?? - AdaptiveSearchTextField( - _controller, - copyFrom: widget.searchBar ?? theme?.searchBar, - padding: widget.searchBarPadding ?? - theme?.searchBarPadding ?? - UiConstants.padding, - showClearButton: - widget.showClearButton ?? theme?.showClearButton ?? true, - )) - : null, + header: _buildHeader(theme),And add a new method:
Widget? _buildHeader(PickerThemeData? theme) { if (!(widget.showHeader ?? theme?.showHeader ?? true)) return null; return widget.header ?? theme?.header ?? AdaptiveSearchTextField( _controller, copyFrom: widget.searchBar ?? theme?.searchBar, padding: widget.searchBarPadding ?? theme?.searchBarPadding ?? UiConstants.padding, showClearButton: widget.showClearButton ?? theme?.showClearButton ?? true, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (2)
packages/world_countries/example/pubspec.lockis excluded by!**/*.lockpackages/world_flags/example/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (39)
.vscode/sealed_world.code-workspace(1 hunks)packages/_sealed_world_tests/pubspec.yaml(1 hunks)packages/analysis_options.yaml(6 hunks)packages/flutter_analysis_options.yaml(1 hunks)packages/sealed_countries/lib/src/model/country/submodels/world_country.dart(1 hunks)packages/sealed_countries/lib/src/model/regional_bloc/regional_bloc.dart(1 hunks)packages/sealed_countries/pubspec.yaml(1 hunks)packages/sealed_currencies/lib/src/model/currency/submodels/fiat_currency.dart(1 hunks)packages/sealed_currencies/pubspec.yaml(1 hunks)packages/sealed_languages/lib/src/helpers/extensions/iso_standardized_string_extension.dart(2 hunks)packages/sealed_languages/lib/src/model/language/submodels/natural_language.dart(1 hunks)packages/sealed_languages/lib/src/model/language_family/submodels/natural_language_family.dart(1 hunks)packages/sealed_languages/lib/src/model/script/submodels/script.dart(2 hunks)packages/sealed_languages/pubspec.yaml(1 hunks)packages/world_countries/example/lib/routing/delegate.dart(1 hunks)packages/world_countries/example/lib/theme/theme_manager.dart(0 hunks)packages/world_countries/example/lib/theme/theme_provider.dart(1 hunks)packages/world_countries/example/lib/widgets/description_tile.dart(1 hunks)packages/world_countries/example/lib/widgets/tab_body.dart(2 hunks)packages/world_countries/example/pubspec.yaml(2 hunks)packages/world_countries/lib/src/extensions/typed_locale_extension.dart(1 hunks)packages/world_countries/lib/src/interfaces/indexed_list_view_interface.dart(0 hunks)packages/world_countries/lib/src/widgets/country/country_picker.dart(1 hunks)packages/world_countries/lib/src/widgets/currency/currency_picker.dart(1 hunks)packages/world_countries/lib/src/widgets/language/language_picker.dart(1 hunks)packages/world_countries/lib/src/widgets/phone_code/phone_code_picker.dart(1 hunks)packages/world_countries/lib/src/widgets/pickers/basic_picker.dart(1 hunks)packages/world_countries/lib/src/widgets/pickers/basic_picker_state.dart(2 hunks)packages/world_countries/pubspec.yaml(1 hunks)packages/world_flags/example/pubspec.yaml(1 hunks)packages/world_flags/lib/src/model/typedefs.dart(1 hunks)packages/world_flags/lib/src/ui/country_flag.dart(1 hunks)packages/world_flags/lib/src/ui/flags/basic_flag.dart(1 hunks)packages/world_flags/lib/src/ui/painters/custom/taegukgi_painter.dart(1 hunks)packages/world_flags/pubspec.yaml(1 hunks)tools/analysis_options.yaml(1 hunks)tools/lib/utils/args_parser.dart(1 hunks)tools/lib/utils/json_utils.dart(2 hunks)tools/pubspec.yaml(2 hunks)
💤 Files with no reviewable changes (2)
- packages/world_countries/example/lib/theme/theme_manager.dart
- packages/world_countries/lib/src/interfaces/indexed_list_view_interface.dart
🧰 Additional context used
📓 Learnings (2)
packages/sealed_currencies/pubspec.yaml (1)
Learnt from: tsinis
PR: tsinis/sealed_world#237
File: packages/sealed_countries/pubspec.yaml:24-24
Timestamp: 2024-11-12T12:26:43.241Z
Learning: `sealed_currencies` version 1.5.1 does not introduce any breaking changes.
packages/sealed_countries/pubspec.yaml (1)
Learnt from: tsinis
PR: tsinis/sealed_world#237
File: packages/sealed_countries/pubspec.yaml:24-24
Timestamp: 2024-11-12T12:26:43.241Z
Learning: `sealed_currencies` version 1.5.1 does not introduce any breaking changes.
🔇 Additional comments (35)
packages/world_flags/lib/src/ui/flags/basic_flag.dart (1)
82-82: LGTM! The parentheses addition improves code clarity.
The addition of parentheses around (ratio ?? _flagAspectRatio) makes the order of operations more explicit without changing the behavior. This is a good practice for improving code readability.
packages/world_flags/lib/src/ui/country_flag.dart (1)
62-62: Documentation alignment confirmed.
Renaming the documentation parameter to alternativeMap brings it into line with the actual constructor parameter name, improving clarity. No issues detected.
tools/pubspec.yaml (3)
12-13: LGTM: Safe patch version updates
The version bumps for collection, path, and yaml packages are patch updates, which typically contain bug fixes and minor improvements without breaking changes.
Also applies to: 19-19
Line range hint 1-29: Verify analysis options after linter removal
The removal of dart_code_linter is noted. To ensure code quality is maintained:
- Confirm that all previously enforced rules are covered by
dart_code_metrics_presets - Update any analysis options files that referenced the removed linter
✅ Verification successful
Removal of dart_code_linter is safe as rules are covered by dart_code_metrics_presets
The verification shows that:
- The project uses
dart_code_metrics_presets/dart_all.yamlwhich provides comprehensive linting coverage - The analysis options files have extensive rule configurations using
dart_code_metricsand standardlinterrules - No direct references to the removed
dart_code_linterpackage were found in the analysis options files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check analysis options files for potential updates needed
echo "Checking analysis_options.yaml files for potential updates:"
fd "analysis_options.yaml" --type f --exec cat {}
Length of output: 25037
28-29: Verify metrics configuration after linter removal
The update to dart_code_metrics_presets (^2.17.0 -> ^2.19.0) and removal of dart_code_linter suggests a consolidation of code analysis tools. Please ensure that all necessary linting rules are covered by the metrics presets.
✅ Verification successful
Let me check the content of these analysis options files to verify the linting configuration.
Linting rules are properly configured after dart_code_linter removal
The analysis of the configuration files shows that all necessary linting rules are properly covered by dart_code_metrics_presets. The package extends dart_code_metrics_presets/dart_all.yaml and has comprehensive configuration for both general Dart rules and Flutter-specific rules. No references to dart_code_linter were found in the codebase, and the metrics configuration is well-maintained with:
- Proper extension of presets
- Detailed rule configurations
- Clear separation between Flutter and Dart rules
- Explicit enable/disable flags for specific rules with documented reasons
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to dart_code_linter
# and verify metrics configuration files
echo "Checking for dart_code_linter references:"
rg "dart_code_linter"
echo -e "\nChecking for metrics configuration files:"
fd "analysis_options|metrics" --type f
Length of output: 548
Script:
#!/bin/bash
# Check content of analysis options files
echo "Content of tools/analysis_options.yaml:"
cat tools/analysis_options.yaml
echo -e "\nContent of packages/analysis_options.yaml:"
cat packages/analysis_options.yaml
echo -e "\nContent of packages/flutter_analysis_options.yaml:"
cat packages/flutter_analysis_options.yaml
Length of output: 25002
packages/world_countries/lib/src/widgets/currency/currency_picker.dart (1)
74-77: Enhanced clarity with explicit parentheses
Using (isDense ?? false) improves readability and ensures the logic is unambiguous. This change appears consistent and correct.
packages/world_countries/lib/src/widgets/language/language_picker.dart (1)
74-77: Consistent use of parentheses for clarity
Adding parentheses around (isDense ?? false) makes the conditional check more explicit, maintaining consistency across similar picker widgets.
packages/world_countries/lib/src/widgets/country/country_picker.dart (1)
117-117: Improved conditional readability
Wrapping the isDense null-check in parentheses is a welcome consistency improvement, aligning this picker with the others.
packages/world_countries/lib/src/widgets/phone_code/phone_code_picker.dart (1)
154-157: Refined conditional checks for better readability
Applying parentheses around (isDense ?? false) for both onPressed and visualDensity echoes the pattern used in other pickers, ensuring uniform clarity everywhere.
packages/world_countries/example/lib/theme/theme_provider.dart (1)
25-26: Ensure the new color method behaves as expected.
Replacing withOpacity with withValues(alpha: 1) is consistent with the new approach to color manipulation. Confirm that all references to this theme return the intended color opacity and that calling code is unaffected.
packages/world_flags/lib/src/model/typedefs.dart (1)
41-43: Documentation updates look good.
The improved formatting and clarity in the documentation for FlagParentBounds help developers understand the record’s fields better. No further changes needed.
Also applies to: 47-47
packages/world_countries/example/lib/widgets/description_tile.dart (1)
49-49: Consistent alpha setting approach.
Switching from withOpacity(0.1) to withValues(alpha: 0.1) aligns with the refactoring throughout the codebase. Good job maintaining a uniform color strategy.
packages/world_countries/lib/src/extensions/typed_locale_extension.dart (1)
95-96: Check for potential breaking changes in tuple ordering.
Reordering currencies and languages in the returned record might cause mismatches in code expecting a specific field order. Ensure downstream consumers correctly reference the changed field order.
packages/sealed_countries/lib/src/model/regional_bloc/regional_bloc.dart (1)
43-43: Use of ignore comment is acceptable.
Marking this constructor as an exception to the lint rule is reasonable here, as it clarifies the implementation intent for a factory constructor. No further changes are required.
.vscode/sealed_world.code-workspace (5)
262-262: Command update aligned with removing old code linter references.
Switching from the verbose dart_code_linter:metrics command to dcm makes the workflow simpler and consistent with the PR objectives.
267-267: Ensure that dcm check-unnecessary-nullable is recognized in your environment.
Please verify that your updated version of dcm supports the check-unnecessary-nullable subcommand to avoid breakage in CI tasks.
272-272: Streamlined lint command.
Using dcm check-unused-l10n is more concise. Ensure that project scripts or documentation referencing the old command are updated accordingly.
277-277: Confirm no custom logic relies on the old metrics check-unused-files.
While this update is straightforward, confirm there are no additional parameters or environment setups required for the new command to function as intended.
282-282: Unused code check command updated.
Switching to dcm check-unused-code is consistent. No immediate issues found.
packages/sealed_languages/lib/src/model/script/submodels/script.dart (2)
55-55: Factory constructor usage is clear and justified.
Continuing to ignore lint rules for non-empty constructor bodies in factory methods is understandable, as it clarifies the usage for code readers. No further changes needed.
105-105: Factory constructor doc is consistent with usage.
The comment explains the intent of ignoring non-empty constructor bodies for clarity in factory methods. This is acceptable.
tools/analysis_options.yaml (1)
8-11: Verify the migration from dart_code_linter to dart_code_metrics
The section rename from dart_code_linter to dart_code_metrics looks good. Let's verify that all related configurations are updated consistently.
✅ Verification successful
Let me check the actual content of these files to verify the migration is complete and consistent.
Migration to dart_code_metrics is complete and consistent
The verification shows that the migration from dart_code_linter to dart_code_metrics is complete. All relevant files are using the new dart_code_metrics configuration consistently:
- Configuration files (
analysis_options.yaml) properly use thedart_code_metricssection - Dependencies in
pubspec.yamlfiles correctly referencedart_code_metrics_presets - No remaining references to
dart_code_linterwere found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining references to dart_code_linter
rg -l "dart_code_linter" --type yaml
# Check for consistent usage of dart_code_metrics
rg -l "dart_code_metrics" --type yaml
Length of output: 511
Script:
#!/bin/bash
# Check the content of yaml files for dart_code_metrics configuration
rg "dart_code_\w+" -B1 -A3 --type yaml
Length of output: 3680
packages/world_flags/example/pubspec.yaml (1)
8-9: Verify SDK version compatibility across dependencies
The SDK version updates look appropriate. However, let's ensure all dependencies are compatible with Dart 3.6.0 and Flutter 3.27.1.
✅ Verification successful
SDK version constraints are compatible across the project
The verification shows that all packages in the repository use Dart SDK version constraints that are compatible with the updated version ^3.6.0:
- Most packages use either ^3.0.0, ^3.3.0, or ^3.6.0
- The lower bounds (^3.0.0) are compatible with the upper version (^3.6.0)
- Flutter version ^3.27.1 is compatible with these Dart SDK versions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for SDK constraints in all pubspec files
fd pubspec.yaml | xargs awk '/sdk:|flutter:/ && /\^3\./{print FILENAME ":" FNR ":" $0}'
# Check for potential version conflicts
fd pubspec.yaml | xargs grep -l "sdk:" | xargs cat | grep "^ sdk:"
Length of output: 902
packages/_sealed_world_tests/pubspec.yaml (1)
14-14: LGTM: Dependencies updated to latest stable versions
All dependency updates are minor/patch version bumps:
- test: ^1.25.7 -> ^1.25.8
- dart_code_metrics_presets: ^2.17.0 -> ^2.19.0
- lints: ^5.0.0 -> ^5.1.1
These updates maintain backward compatibility while incorporating the latest improvements.
Also applies to: 18-19
packages/sealed_languages/pubspec.yaml (1)
26-30: LGTM: Dev dependencies updated consistently
All updates are to development dependencies and maintain backward compatibility:
- Minor version bumps: coverage (1.9 -> 1.11)
- Patch version bumps: build_runner, dart_code_metrics_presets, lints, test
The updates align with the changes in _sealed_world_tests package.
packages/world_flags/pubspec.yaml (1)
27-27: LGTM: Consistent dependency updates across packages
The dev dependency updates align with changes in other packages:
- dart_code_metrics_presets: ^2.19.0
- lints: ^5.1.1
Also applies to: 31-31
✅ Verification successful
Dependency versions are consistent across all packages
The verification confirms that all packages in the repository use consistent versions:
dart_code_metrics_presets: ^2.19.0in all packages including toolslints: ^5.1.1in all packages with the same comment "# From Google"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistency of these dependencies across all packages
echo "Checking dart_code_metrics_presets versions:"
fd -e yaml pubspec.yaml -x grep -H "dart_code_metrics_presets:" {}
echo -e "\nChecking lints versions:"
fd -e yaml pubspec.yaml -x grep -H "lints:" {}
Length of output: 1547
packages/sealed_countries/pubspec.yaml (1)
29-33: LGTM! Dev dependency updates look safe.
All updates are minor version bumps following semantic versioning, maintaining backward compatibility. The changes align with the PR objective of updating dependencies.
Let's verify these versions are the latest stable releases:
✅ Verification successful
Let me try a different approach using the Dart pub API to check the package versions.
✅ All dev dependencies are up-to-date with the latest stable versions
Verification confirms that all package versions in pubspec.yaml match the latest stable versions from pub.dev:
- build_runner: ^2.4.14 matches latest 2.4.14
- coverage: ^1.11.1 matches latest 1.11.1
- dart_code_metrics_presets: ^2.19.0 matches latest 2.19.0
- lints: ^5.1.1 matches latest 5.1.1
- test: ^1.25.14 matches latest 1.25.14
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify latest stable versions of dev dependencies
# Note: Using GitHub API to check the latest releases
for package in "build_runner" "coverage" "dart_code_metrics_presets" "lints" "test"; do
echo "Checking $package..."
gh api graphql -f query='{
repository(owner: "dart-lang", name: "'$package'") {
releases(last: 1) {
nodes {
tagName
isPrerelease
}
}
}
}'
done
Length of output: 2529
Script:
#!/bin/bash
# Description: Check latest versions using pub.dev API
for package in "build_runner" "coverage" "dart_code_metrics_presets" "lints" "test"; do
echo "Checking $package..."
curl -s "https://pub.dev/api/packages/$package" | jq -r '.latest.version'
done
Length of output: 1121
packages/sealed_currencies/pubspec.yaml (1)
29-33: LGTM! Consistent with sealed_countries package.
The dev dependency updates mirror those in the sealed_countries package, maintaining consistency across the project.
packages/world_countries/pubspec.yaml (1)
44-45: LGTM! Dev dependency updates align with other packages.
The updates maintain consistency with other packages in the project.
Also applies to: 49-49
packages/world_countries/example/lib/routing/delegate.dart (1)
23-23: LGTM: Well-documented lint ignore.
The added ignore comment is appropriate and well-documented, explaining that the non-empty constructor body comes from the original Google code. This maintains consistency with the source code while acknowledging the lint rule.
packages/world_countries/example/lib/widgets/tab_body.dart (1)
29-30: LGTM: More explicit color opacity handling.
The change from withOpacity to withValues(alpha:) makes the code more explicit about which color component is being modified. Both methods achieve the same result, and the new approach aligns better with modern Dart practices.
Also applies to: 58-59
packages/flutter_analysis_options.yaml (1)
11-18: LGTM! Widget-specific rules are well configured.
The configuration for widget-related rules is appropriate:
avoid-returning-widgetswithallow-nullable: truehelps prevent widget tree rebuilds while allowing null returnsprefer-widget-private-memberswithignore-static: truepromotes encapsulation while allowing static utility methods
packages/world_flags/lib/src/ui/painters/custom/taegukgi_painter.dart (1)
120-120: LGTM! Well-documented lint ignore.
The ignore comment for prefer-for-in is appropriate and well-documented, as the index is needed for calculations in the loop.
packages/sealed_languages/lib/src/helpers/extensions/iso_standardized_string_extension.dart (2)
111-118: LGTM! Code readability improvements.
The changes enhance code clarity by adding parentheses around conditions and callback invocations while maintaining the same functionality.
130-131: LGTM! Consistent callback invocation formatting.
The changes maintain consistent formatting with parentheses around callback invocations, aligning with the codebase style.
packages/sealed_languages/lib/src/model/language_family/submodels/natural_language_family.dart
Show resolved
Hide resolved
packages/sealed_countries/lib/src/model/country/submodels/world_country.dart
Show resolved
Hide resolved
packages/sealed_currencies/lib/src/model/currency/submodels/fiat_currency.dart
Show resolved
Hide resolved
packages/sealed_languages/lib/src/model/language/submodels/natural_language.dart
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/world_flags/test/goldens/simplified/mda.pngis excluded by!**/*.png
📒 Files selected for processing (13)
.github/readme.md(1 hunks).github/workflows/publish_world_countries.yaml(0 hunks).github/workflows/publish_world_flags.yaml(1 hunks).vscode/sealed_world.code-workspace(2 hunks)packages/_sealed_world_tests/README.md(1 hunks)packages/sealed_countries/README.md(1 hunks)packages/sealed_currencies/README.md(1 hunks)packages/sealed_languages/README.md(1 hunks)packages/world_countries/README.md(2 hunks)packages/world_flags/README.md(1 hunks)packages/world_flags/lib/src/model/elements/elements_properties.dart(1 hunks)packages/world_flags/test/src/model/colors_properties_test.dart(1 hunks)packages/world_flags/test/src/model/elements/elements_properties_test.dart(1 hunks)
💤 Files with no reviewable changes (1)
- .github/workflows/publish_world_countries.yaml
🧰 Additional context used
📓 Learnings (1)
packages/sealed_languages/README.md (1)
Learnt from: tsinis
PR: tsinis/sealed_world#235
File: packages/sealed_languages/README.md:3-4
Timestamp: 2024-11-12T12:26:43.241Z
Learning: In the README files, badges like CodeFactor and Codecov are implicit and not manually created. Do not suggest updating them for consistency.
🪛 Markdownlint (0.37.0)
.github/readme.md
14-14: Element: a
Inline HTML
(MD033, no-inline-html)
15-15: Element: img
Inline HTML
(MD033, no-inline-html)
packages/world_countries/README.md
52-52: Element: a
Inline HTML
(MD033, no-inline-html)
53-53: Element: img
Inline HTML
(MD033, no-inline-html)
🪛 yamllint (1.35.1)
.github/workflows/publish_world_flags.yaml
[warning] 3-3: truthy value should be one of [false, true]
(truthy)
[error] 7-7: wrong indentation: expected 6 but found 4
(indentation)
[error] 16-16: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (17)
.vscode/sealed_world.code-workspace (6)
3-6: Verify folder path correctness
The path has changed from "../.vscode" to ".". Ensure that project references, settings, and any dependencies still point to the correct folder after this change.
268-268: Simplify linting command
Switching from dart run dart_code_linter:metrics analyze lib to dcm analyze lib makes the lint command more concise and aligns with the new dcm package usage. This looks good.
273-273: Command update for unnecessary nullable check
Changing to dcm check-unnecessary-nullable lib is consistent with the new, shorter command usage. Nice improvement.
278-278: Refined command for checking unused l10n
Using dcm check-unused-l10n lib is succinct and standardized across metrics commands.
283-283: Simplify checking for unused files
Improving the command to dcm check-unused-files lib is consistent with the updated metrics approach.
288-288: New command for checking unused code
Rewriting the command to dcm check-unused-code lib ensures consistency with the new tools. Straightforward change.
packages/_sealed_world_tests/README.md (1)
5-6: LGTM! Badge improvements enhance documentation accessibility.
The addition of hyperlinks to the badges improves user experience by providing direct access to relevant GitHub pages.
.github/readme.md (2)
5-6: LGTM! Enhanced badge accessibility.
The addition of hyperlinks to the badges improves user experience by providing direct access to relevant GitHub pages.
13-15: LGTM! Development environment badges enhance developer onboarding.
The addition of Codespaces and IDX badges provides quick access to development environments. The use of inline HTML for the IDX badge is justified as it requires specific styling that cannot be achieved with standard Markdown.
🧰 Tools
🪛 Markdownlint (0.37.0)
14-14: Element: a
Inline HTML
(MD033, no-inline-html)
15-15: Element: img
Inline HTML
(MD033, no-inline-html)
packages/world_flags/README.md (1)
6-8: LGTM! Badge improvements enhance package metrics visibility.
The addition of hyperlinks to the badges improves user experience by providing direct access to package metrics and repository information.
packages/sealed_languages/README.md (1)
6-9: LGTM: Badge updates look good!
The badge links are properly formatted and point to the correct resources.
packages/sealed_currencies/README.md (1)
6-9: LGTM: Badge updates look good!
The badge links are properly formatted and point to the correct resources.
packages/world_countries/README.md (1)
6-9: LGTM: Badge updates look good!
The badge links are properly formatted and point to the correct resources.
packages/sealed_countries/README.md (1)
6-9: LGTM! Documentation improvements.
The changes enhance the README by making the badges clickable, providing direct access to relevant metrics and resources. This improves the documentation's usability by allowing users to easily access:
- Package scoring on pub.dev
- Latest commit information
- Package popularity metrics
- GitHub repository stars
.github/workflows/publish_world_flags.yaml (2)
10-16: LGTM! Secure job configuration.
The job configuration follows security best practices:
- Uses local workflow reference
- Follows principle of least privilege with minimal permissions
- Correctly passes required parameters
🧰 Tools
🪛 yamllint (1.35.1)
[error] 16-16: no new line character at the end of file
(new-line-at-end-of-file)
11-11: Verify the reused workflow configuration.
Let's verify the existence and configuration of the reused workflow file.
✅ Verification successful
Reused workflow configuration is properly set up
The reused workflow file .github/workflows/publish_to_pub.yaml exists and is well-configured with:
- Required inputs:
package_nameandtag - Proper version checking across pubspec.yaml, CHANGELOG.md, and git tag
- Safety checks to ensure versions match before publishing
- Appropriate publishing configuration using the official Dart workflow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the reused workflow file exists and check its configuration
# Check if the workflow file exists and show its contents
if [[ -f .github/workflows/publish_to_pub.yaml ]]; then
echo "Workflow file exists. Contents:"
cat .github/workflows/publish_to_pub.yaml
else
echo "Warning: Referenced workflow file .github/workflows/publish_to_pub.yaml does not exist"
fi
Length of output: 2690
packages/world_flags/test/src/model/colors_properties_test.dart (1)
15-17: Confirm string representation stability.
The updated test string is now more explicit about the color properties, which can be helpful but may vary if Flutter's Color or MaterialColor implementations change their toString() output in future versions. Consider a more robust approach (for example, verifying only the essential parts of the color representation) to reduce potential false positives in testing.
Below is a shell script that searches for other exact expectations of Color(...) or MaterialColor(...) in the tests, ensuring they match your new format:
packages/world_flags/test/src/model/elements/elements_properties_test.dart
Show resolved
Hide resolved
Description
Type of Change
Checks
Please look at the following checklist to ensure that your PR can be accepted quickly:
Summary by CodeRabbit
New Features
Scriptclass for improved instance creation.Bug Fixes
Documentation
Chores