Skip to content

[feature] Add per-repo allowlist for direct-change attribute detection - #273

Merged
yushan8 merged 4 commits into
mainfrom
yushan/add-directly-changed-attributes-filter
Aug 6, 2026
Merged

[feature] Add per-repo allowlist for direct-change attribute detection#273
yushan8 merged 4 commits into
mainfrom
yushan/add-directly-changed-attributes-filter

Conversation

@yushan8

@yushan8 yushan8 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Intent:

  • GetChangedTargets classifies a target as directly changed (distance 0)
    whenever its attribute set differs between revisions.
  • Some Bazel attributes are pure BUILD-file bookkeeping (e.g.
    generator_location, which shifts whenever unrelated lines move earlier in
    the same file) rather than semantic configuration, so comparing the full
    attribute set can misclassify cosmetic churn as a direct change.

Changes:

  • Add RepositoryConfig.DirectlyChangedAttributes, a per-repo allowlist of
    attribute names that count as evidence of a direct change.
  • Wire config.RepositoryConfigProvider into controller.Params/controller so
    GetChangedTargets can look up the allowlist for a request's remote.
  • Thread the resolved allowlist through compareTargetGraphs and toDiffGraph
    so only allowlisted attributes (or all attributes, when unset) are kept on
    each target before diffing.

Test Plan

  • Added TestToDiffGraph_OnlyKeepsAllowlistedAttributes and
    TestDirectlyChangedAttributesFor covering the allowlist filtering and
    lookup logic.
  • Updated existing compareTargetGraphs/toDiffGraph call sites for the new
    parameter.

@yushan8
yushan8 marked this pull request as ready for review August 6, 2026 16:58
@yushan8
yushan8 requested review from a team as code owners August 6, 2026 16:58
Comment thread config/repository_config.go Outdated
yushan8 added 2 commits August 6, 2026 10:45
Summary:
Intent:
- GetChangedTargets classifies a target as directly changed (distance 0)
  whenever its attribute set differs between revisions.
- Some Bazel attributes are pure BUILD-file bookkeeping (e.g.
  generator_location, which shifts whenever unrelated lines move earlier in
  the same file) rather than semantic configuration, so comparing the full
  attribute set can misclassify cosmetic churn as a direct change.

Changes:
- Add RepositoryConfig.DirectlyChangedAttributes, a per-repo allowlist of
  attribute names that count as evidence of a direct change.
- Wire config.RepositoryConfigProvider into controller.Params/controller so
  GetChangedTargets can look up the allowlist for a request's remote.
- Thread the resolved allowlist through compareTargetGraphs and toDiffGraph
  so only allowlisted attributes (or all attributes, when unset) are kept on
  each target before diffing.

Test Plan:
- Added TestToDiffGraph_OnlyKeepsAllowlistedAttributes and
  TestDirectlyChangedAttributesFor covering the allowlist filtering and
  lookup logic.
- Updated existing compareTargetGraphs/toDiffGraph call sites for the new
  parameter.

---

<sub>Generated by the 🪄 [pr-create](https://sg.uberinternal.com/code.uber.internal/uber-non-production/personal-wua-skills/-/blob/pr-create/SKILL.md) skill in wua/skills</sub>
Reviewer feedback: the original name read unclear about what it controlled.
Renamed the field, YAML tag, and internal identifiers to SeedAttributes /
seed_attributes for clarity.
@yushan8
yushan8 force-pushed the yushan/add-directly-changed-attributes-filter branch from 3931d7a to 5b8a99f Compare August 6, 2026 17:46
@yushan8 yushan8 closed this Aug 6, 2026
@yushan8 yushan8 reopened this Aug 6, 2026
@yushan8
yushan8 marked this pull request as draft August 6, 2026 20:51
@yushan8
yushan8 marked this pull request as ready for review August 6, 2026 20:51
@yushan8
yushan8 merged commit f45ed6c into main Aug 6, 2026
3 checks passed
@yushan8
yushan8 deleted the yushan/add-directly-changed-attributes-filter branch August 6, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants