Add custom properties by Sandijigs · Pull Request #2512 · rust-lang/team

Sandijigs · 2026-06-09T00:42:54Z

Adds support for [custom-properties] in the team repo's TOML, and sets crabwatch = true on rust-lang/crabwatch.
Booleans only for now. Properties on a repo but not declared in TOML are left alone.
Closes #2504.

rustbot · 2026-06-09T00:42:57Z

rust_team_data/src/v1.rs has been modified, it is used (as a git dependency) by multiple sub-projects like triagebot, the www.rust-lang.org website and others.

If you are changing the data structures, please make sure that the changes are not going to break serde deserialization (adding a field is fine; removing or renaming a field isn't).

If you must do a breaking change to the format, make sure to coordinate it with all the users of the rust_team_data crate.

github-actions · 2026-06-09T00:46:09Z

Dry-run check results


[WARN  rust_team::sync] sync-team is running in dry mode, no changes will be applied.
[INFO  rust_team::sync] synchronizing crates-io
[INFO  rust_team::sync] synchronizing github

ubiratansoares

@Sandijigs thanks for this PR, looks we are in the right direction here. Added a few comments :)

ubiratansoares · 2026-06-10T18:51:57Z

+            let operation = match actual_by_name.get(name) {
+                // Missing on the repo, or value is null.
+                None | Some(None) => CustomPropertyDiffOperation::Create(expected),
+                Some(Some(actual)) if actual != &expected => {
+                    CustomPropertyDiffOperation::Update(actual.clone(), expected)
+                }
+                Some(Some(_)) => continue,
+            };


Perhaps is room to make this code more concise with some pattern matching tricks. I'd recommend having a look on how to change

let operation = match actual_by_name.get(name)

in order to get rid of the nested Option value :)

Refactored to flatten the nested Option using .and_then(|v| v.as_deref()).
The match now has three flat arms instead of three nested ones.

ubiratansoares · 2026-06-10T18:52:54Z

-  "auto_merge_enabled": true
+  "auto_merge_enabled": true,
+  "custom_properties": {}
 }


It's probaly a good idea adding an empty line in these files, so we keep the standard

Updated the JSON writer in static_api.rs to add a trailing newline.

ubiratansoares · 2026-06-10T18:53:47Z

        Ok(ruleset_diffs)
    }

+    async fn diff_custom_properties(


I missed some tests that actually prove this function works as intended in sync/github/tests/mod.rs.

I propose adding at least two cases :

adding a propery to a repo

removing a property from a repo

Added two tests, one for adding a property and one for removing one as suggested. Both run through the diff function end to end. Also extended the mock so tests can set what's on the GitHub side independently from the TOMLside (similar to how rulesets/environments work).

ubiratansoares · 2026-06-10T18:55:40Z

    // https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request
    pub auto_merge_enabled: bool,
+    #[serde(default)]
+    pub custom_properties: BTreeMap<String, bool>,


In general we've been using IndexMap to represent key-value pairs in this file. Do you have any particular reason to go with BTreeMap instead?

Actually there is no strong reason. BTreeMap gives deterministic key ordering, but IndexMap fits the file's convention better. I will switched it

Kobzol

Left a few comments :)

Kobzol · 2026-06-10T19:02:09Z

    // Is the GitHub "Auto-merge" option enabled?
    // https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/incorporating-changes-from-a-pull-request/automatically-merging-a-pull-request
    pub auto_merge_enabled: bool,
+    #[serde(default)]


The #[serde(default)] annotation is not necessary, because the generator (the team API CI) will be updated sooner than the consumers (the crates that depend on the team crate).

Removed. Makes sense given the generator updates before consumers.

Kobzol · 2026-06-10T19:05:04Z

+#[derive(Debug)]
+enum CustomPropertyDiffOperation {
+    Create(String),
+    Update(String, String), // old, new


This should perhaps also contain Delete, in case the custom property is removed from the team config.

It would mean that team is the ground truth for all our custom properties, although it would also delete all custom properties that are not set in team.

@ubiratansoares Do you want team to be the ground truth for all custom properties? If it is not, then once a property is created through team, it won't ever be deleted again.

Do you want team to be the ground truth for all custom properties?

That's an interesting question, my gut feeling says yes, since I see these custom properties as tool to enable governance-related automations over Github repos or Github orgs, and team is the place where we want represent and enable such stuff with IaC.

I agree, but we should be then careful with the first sync (or ideally a dry-run executed manually) to ensure that we don't delete any existing custom properties; those would have to be backported into team first.

I'll check whether there are any custom properties out there, so we don't break anything by mistake here

Added the Delete variant. The diff emits a. Delete for any property on GitHub that isn't declared in TOML, so team becomes the source of truth. The apply sends a null value through the existing PATCH endpoint, which is GitHub's way of unsetting.

I'll check whether there are any custom properties out there, so we don't break anything by mistake

@Sandijigs @Kobzol just to confirm : I ran a custom script and confirmed that no repos actually use custom properties (for rust-lang org)

Ok, good to know. In that case we can directly switch to using team as a source of truth (unless repos in some of our other orgs use them.. :) ).

Kobzol · 2026-06-10T19:05:48Z

+# Repository custom properties (optional)
+[custom-properties]
+# Set a property name to a boolean value
+crabwatch = true


The property values on GitHub are strings, and the code currently converts bools to a string via its Display impl, which is a bit opaque and potentially fragile. Maybe we could just expose the value as a string?

That's a fair point. The reason I went with bool is that @marcoieni suggested it earlier. TOML supports it natively, and the conversion to string happens on the Rust side.
I can switch to strings throughout if you'd rather, but probably worthchecking with @marcoieni first so we don't end up bouncing between the two.

Yeah, I was wrong on this. The fact that the property crabwatch can be only true or false isn't true for all properties, so it makes sense to use strings. Sorry about this!

Sandijigs added 4 commits June 9, 2026 00:41

Add custom_properties field to Repo data types

467dfe5

Sync custom properties through the GitHub API

a9dc003

Document the custom-properties TOML field

6d2bb0d

Set crabwatch custom property on rust-lang/crabwatch

cdb81bc

Sandijigs requested review from Mark-Simulacrum, emilyalbini, jdno, marcoieni and ubiratansoares as code owners June 9, 2026 00:42

jieyouxu added needs-infra-admin-review This change requires one of the `infra-admins` to review. S-waiting-on-review Status: waiting on review from a team/WG/PG lead, an infra-admin, and/or a team-repo-admin. labels Jun 10, 2026

Sandijigs mentioned this pull request Jun 10, 2026

Added basic clap CLI with subcommand stubs rust-lang/crabwatch#5

Merged

ubiratansoares reviewed Jun 10, 2026

View reviewed changes

Kobzol reviewed Jun 10, 2026

View reviewed changes

Apply PR review feedback

2c292de

Conversation

Sandijigs commented Jun 9, 2026

Uh oh!

rustbot commented Jun 9, 2026

Uh oh!

github-actions Bot commented Jun 9, 2026

Uh oh!

ubiratansoares left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sandijigs Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sandijigs Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kobzol left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ubiratansoares Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sandijigs Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Sandijigs Jun 14, 2026 •

edited

Loading

Sandijigs Jun 14, 2026 •

edited

Loading

ubiratansoares Jun 15, 2026 •

edited

Loading

Sandijigs Jun 14, 2026 •

edited

Loading