feat(repo-policy-sync): manage Bazel dev dependencies - #34
Conversation
Manual plan result:
|
There was a problem hiding this comment.
🟡 Changes recommended
The operation can emit invalid Bazel syntax around trailing comments, and the policy misclassifies a dependency required by a public macro.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds reusable Bazel dev_dependency alignment to repository policies.
Changes:
- Adds and registers a new dependency operation.
- Introduces the bundled alignment policy and fixtures.
- Adds tests and reference documentation.
File summaries
| File | Description |
|---|---|
repo_policy_sync/tests/operations/test_ensure_bazel_dependency_dev_dependency.py |
Tests parsing and transformations. |
repo_policy_sync/src/operations/README.md |
Documents the operation. |
repo_policy_sync/src/operations/ensure_bazel_dependency_dev_dependency.py |
Implements dependency alignment. |
repo_policy_sync/src/operations/__init__.py |
Registers the operation. |
repo_policy_sync/src/models.py |
Adds the operation model. |
repo_policy_sync/policies/README.md |
Lists the new policy. |
repo_policy_sync/policies/dev-dependency-alignment/policy.yml |
Defines dependency classifications. |
repo_policy_sync/policies/dev-dependency-alignment/no-target-dependencies/before/MODULE.bazel |
Adds absent-target input fixture. |
repo_policy_sync/policies/dev-dependency-alignment/no-target-dependencies/after/MODULE.bazel |
Adds absent-target expected fixture. |
repo_policy_sync/policies/dev-dependency-alignment/mixed-dependencies/before/MODULE.bazel |
Adds mixed-dependency input fixture. |
repo_policy_sync/policies/dev-dependency-alignment/mixed-dependencies/after/MODULE.bazel |
Adds aligned expected fixture. |
repo_policy_sync/docs/reference/policy-format.md |
Documents configuration and behavior. |
Review details
- Files reviewed: 12/12 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
copilot findings not relevant / incorrect |
MaximilianSoerenPollak
left a comment
There was a problem hiding this comment.
Judging by the tests, it seems to work fine as is.
Waiting for copilot review to see something that I overlooked.
Not a fan of the regex but there is not much else that can be used here.
| @@ -0,0 +1,260 @@ | |||
| # ******************************************************************************* | |||
There was a problem hiding this comment.
Might be worth using the same logic as far as possible that we use in bazel_registry, so we don't have two regex that behave differently.
Though unsure if they are too far apart in usage.
| _MODULE_FILE = Path("MODULE.bazel") | ||
| _MODULE_NAME = re.compile(r"[A-Za-z0-9_][A-Za-z0-9_.-]*\Z") | ||
| _NAME_ARGUMENT = re.compile(r"\bname\s*=\s*[\"']([^\"']+)[\"']") | ||
| _DEV_DEPENDENCY_ARGUMENT = re.compile(r"\bdev_dependency\s*=\s*(True|False)\b") |
There was a problem hiding this comment.
I know this will come back one day to bite us.
|
Addressed the actionable feedback in commit The operation could place the comma inside a trailing inline comment when adding I also centralized the shared Bazel module-name and The Validation passed: 302 tests, Ruff, pre-commit hooks, and |
Why
This PR adds reusable policy support for keeping selected Bazel dependencies in the correct development graph. This is necessary because expressing
dev_dependencychanges as repository-wide regular-expression policies is fragile: the same rule must safely handle inline and multilinebazel_depcalls, existing boolean values, comments, and different subsets of dependencies across repositories.Review
Please focus on the module set and whether I marked them correctly as dev or non dev dependencies. This is not meant to be complete. But it is meant to be correct.
What
ensure_bazel_dependency_dev_dependencyoperation for the repository-rootMODULE.bazel;dev_dependency: trueadds or enables the attribute, whilefalseremoves it.dev-dependency-alignmentpolicy. It requiresscore_bazel_cpp_toolchains,aspect_rules_py,buildifier_prebuilt,aspect_rules_lint,toolchains_llvm, andscore_rules_imagefsto be development-only, while keepingscore_docs_as_codein the normal dependency graph.any_direct_module_dependencies.