Configure format and analyze hooks via YAML#148
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the Git hooks by renaming BaseGitHook to BaseHook and introducing a configuration mechanism via dart_hooks.yaml using the yaml package. Both DartAnalyzeHook and DartFormatHook have been updated to support configuration keys, and tests have been updated and expanded accordingly. The code review feedback highlights two main issues: first, exiting with 1 on repository root or status failures violates the orchestrator integration contract, which expects an exit code of 0 to parse the JSON decision; second, using synchronous file reading (readAsStringSync) blocks the Dart event loop, and it is recommended to use non-blocking asynchronous reading with FutureOr<String> instead.
…it/Status exit codes to 0
…t endsWith checks
…arators on Windows
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to conditionally enable or disable Dart format and analyze hooks using a dart_hooks.yaml configuration file. It renames BaseGitHook to BaseHook, updates the documentation, and adapts the tests to support the new configuration. A critical issue was identified in both agent_dart_analyze.dart and agent_dart_format.dart where resolving the package root using Directory.current.parent.path fails when the hooks are run manually from the project root, causing them to look for the configuration file in the wrong directory and exit silently.
Bump dart_hooks to flutter/skills#148 (v0.1.0), which disables the format and analyze hooks by default unless a dart_hooks.yaml config file is present at the repo root with the hook enabled. - Bump the dart_hooks git ref to the flutter#148 merge commit and re-resolve pubspec.lock (no other dependencies changed). - Gitignore dart_hooks.yaml and .agents/*.log so enabling the hooks is a deliberate, per-developer opt-in step and generated logs stay untracked. Developers enable the hooks locally by creating dart_hooks.yaml at the repo root with: DartFormatHook: true DartAnalyzeHook: true
…#151) * Fix dart_hooks.yaml example keys so hooks are actually enabled (#150) The example dart_hooks.yaml files committed in #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. * Update tool/dart_hooks/lib/src/base_hook.dart Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * Remove redundant cast that failed dart analyze --fatal-infos 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. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 participant
Adds configuration checking to hooks, enabling selective format and analyze runs based on dart_hooks.yaml.
This is needed so we can make hooks opt in and the hooks spec does not have a way to have hooks opt in only.
Additionally this fixes the reference to git hooks which are a different thing.
Fixes #140