Skip to content

Update dart_skills_lint dependency to e449787 and optimize skills validation test#186528

Open
reidbaker wants to merge 2 commits into
flutter:masterfrom
reidbaker:update-dart-skills-lint-2026-05-14
Open

Update dart_skills_lint dependency to e449787 and optimize skills validation test#186528
reidbaker wants to merge 2 commits into
flutter:masterfrom
reidbaker:update-dart-skills-lint-2026-05-14

Conversation

@reidbaker
Copy link
Copy Markdown
Contributor

@reidbaker reidbaker commented May 14, 2026

Updates the pinned Git reference for dart_skills_lint in dev/tools/pubspec.yaml to commit hash e4497873950727ee781fa411c1a2f624b1ec50c6 (version 0.3.0).

Introduces a standalone dev/tools/dart_skills_lint.yaml configuration file and refactors dev/tools/test/validate_skills_test.dart to dynamically load and enforce this centralized configuration.
All default built-in rules are explicitly disabled within the secondary backticks validation test block for performance.
References flutter/skills#85

Pre-launch Checklist

@reidbaker reidbaker requested a review from jtmcdole as a code owner May 14, 2026 15:36
@flutter-dashboard flutter-dashboard Bot added the CICD Run CI/CD label May 14, 2026
Copy link
Copy Markdown
Contributor

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

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 introduces a configuration file for dart_skills_lint and updates validate_skills_test.dart to utilize it, alongside a dependency update. Feedback suggests resolving the configuration path relative to the repository root to avoid fragility, removing redundant directory parameters already defined in the configuration, and implementing a more maintainable method for disabling lint rules.

Comment thread dev/tools/test/validate_skills_test.dart Outdated
},
);
final Configuration config = await ConfigParser.loadConfig(path: _configFilePath);
final bool isValid = await validateSkills(skillDirPaths: [skillsDirectory], config: config);
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

The skillDirPaths argument appears redundant here because the config object (loaded from dart_skills_lint.yaml) already specifies the target directory (.agents/skills). Providing both might lead to the same directory being scanned twice, which contradicts the goal of optimizing validation overhead. Consider removing the explicit skillDirPaths if the configuration is intended to be the source of truth for the validation.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ConfigParser doesn't have .agent/skills in it...

Comment thread dev/tools/test/validate_skills_test.dart
jtmcdole
jtmcdole previously approved these changes May 14, 2026
@github-actions github-actions Bot removed the CICD Run CI/CD label May 14, 2026
@reidbaker reidbaker added the CICD Run CI/CD label May 14, 2026
@reidbaker reidbaker requested a review from jtmcdole May 14, 2026 19:20
@reidbaker reidbaker added the autosubmit Merge PR when tree becomes green via auto submit App label May 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App CICD Run CI/CD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants