Skip to content

Migrate tool/generator test to ConfigParser.loadConfig and clean up workflow#185

Closed
reidbaker-agent wants to merge 1 commit into
flutter:mainfrom
reidbaker-agent:migrate-generator-skills-test-2026-07-20
Closed

Migrate tool/generator test to ConfigParser.loadConfig and clean up workflow#185
reidbaker-agent wants to merge 1 commit into
flutter:mainfrom
reidbaker-agent:migrate-generator-skills-test-2026-07-20

Conversation

@reidbaker-agent

@reidbaker-agent reidbaker-agent commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Migrates tool/generator/test/lint_skills_test.dart to use ConfigParser.loadConfig and creates tool/generator/dart_skills_lint.yaml.

  • Test Migration: Updated validateSkills call in tool/generator/test/lint_skills_test.dart to load configuration dynamically via ConfigParser.loadConfig().
  • Workflow Cleanup: Removed duplicate validate_skills CLI job from .github/workflows/dart_skills_lint_workflow.yaml.
  • Verification: Executed dart analyze --fatal-infos (0 issues) and dart test (100% tests passed).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the lint_skills_test.dart file to use resolvedRuleConfigs with RuleConfigPatch instances instead of resolvedRules. The feedback suggests making the entire map literal const to avoid redundant const keywords on individual RuleConfigPatch instances, which improves readability and aligns with Dart best practices.

Comment on lines 22 to 29
resolvedRuleConfigs: {
'check-relative-paths': const RuleConfigPatch(
severity: AnalysisSeverity.error,
),
'check-absolute-paths': const RuleConfigPatch(
severity: AnalysisSeverity.error,
),
},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

In Dart, when a collection literal is marked as const, all nested constructor calls and literals are implicitly const as well. You can make the entire map literal const and omit the redundant const keywords on individual RuleConfigPatch instances. This improves readability and ensures the entire map is a compile-time constant.

Suggested change
resolvedRuleConfigs: {
'check-relative-paths': const RuleConfigPatch(
severity: AnalysisSeverity.error,
),
'check-absolute-paths': const RuleConfigPatch(
severity: AnalysisSeverity.error,
),
},
resolvedRuleConfigs: const {
'check-relative-paths': RuleConfigPatch(
severity: AnalysisSeverity.error,
),
'check-absolute-paths': RuleConfigPatch(
severity: AnalysisSeverity.error,
),
},
References
  1. Effective Dart recommends preferring const over const constructor calls. When a collection literal is prefixed with const, nested constructor calls and literals are implicitly const and do not need the const keyword. (link)

@reidbaker-agent
reidbaker-agent force-pushed the migrate-generator-skills-test-2026-07-20 branch from aec31fb to 66146d5 Compare July 20, 2026 20:12
@reidbaker-agent
reidbaker-agent force-pushed the migrate-generator-skills-test-2026-07-20 branch from 66146d5 to fce05cc Compare July 20, 2026 20:13
@reidbaker reidbaker closed this Jul 20, 2026
@reidbaker reidbaker changed the title Migrate tool/generator test off deprecated resolvedRules onto resolvedRuleConfigs Migrate tool/generator test to ConfigParser.loadConfig and clean up workflow Jul 20, 2026
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