Fix dart_hooks.yaml example keys so hooks are actually enabled (#150)#151
Merged
Merged
Conversation
…er#150) The example dart_hooks.yaml files committed in flutter#148 used the script filenames (agent_dart_format.dart / agent_dart_analyze.dart) as keys. BaseHook.run() gates on each hook's configKey, which is the class name (DartFormatHook / DartAnalyzeHook), so copying these files left both hooks silently disabled, contradicting the README. Update all four committed dart_hooks.yaml files to the class-name keys the code reads and the README documents. Also improve the diagnostic: when the expected key is missing, the log now lists the keys that were found and suggests the correct one, so a typo'd or legacy key no longer disables a hook with an opaque message.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the configuration keys in several dart_hooks.yaml files from script filenames to class-based hook names (DartFormatHook and DartAnalyzeHook). It also improves the logging in base_hook.dart when a hook is disabled due to a missing configuration key by listing the keys that were found. A review comment points out that since yaml is declared as dynamic, accessing yaml.keys results in a dynamic invocation. It is recommended to cast yaml to Map to avoid potential linter warnings.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
yaml is already promoted to Map by the enclosing `if (yaml is Map)`, so `(yaml as Map)` is an unnecessary_cast that `dart analyze --fatal-infos` treats as fatal. Drop the cast.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #150.
The example
dart_hooks.yamlfiles committed in #148 used the script filenames as keys (agent_dart_format.dart/agent_dart_analyze.dart).BaseHook.run()gates each hook on itsconfigKey, which is the class name (DartFormatHook/DartAnalyzeHook). Because those keys never matched, copying any committed example left both hooks silently disabled — contradicting the README, which documents the correct keys.Changes
Update all four committed
dart_hooks.yamlfiles to the class-name keys the code reads and the README documents:dart_hooks.yamltool/dart_hooks.yamltool/dart_hooks/dart_hooks.yamltool/dart_skills_lint/dart_hooks.yaml(a fourth file with the same bug, not listed in the issue)Improve the diagnostic (the issue's optional suggestion). When the expected key is missing, the log now lists the keys that were found and suggests the correct one, so a typo'd or legacy key no longer disables a hook with an opaque message:
This is scoped to each hook's own key (no shared-registry refactor) since the message points straight at the fix. The README troubleshooting section quotes only the unchanged message prefix, so it stays accurate.
Verification
dart test— all pass (extended the existing "missing config key" test to assert the found-keys diagnostic)dart format --set-exit-if-changed— cleandart analyze— no issues