change prevent-update default

[marcoieni]

Discussed in #2327 (comment)

The dry run sets "Restrict updates: true" to all rulesets that have pr-required: true, which I think is good.

[github-actions]

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
[INFO  rust_team::sync] 💻 Repo Diffs:
    📝 Editing repo 'rust-lang/bors':
      Rulesets:
          Updating 'main'
            Restrict updates: true
    📝 Editing repo 'rust-lang/cargo':
      Rulesets:
          Updating 'master'
            Restrict updates: true
          Updating 'rust-1.*'
            Restrict updates: true
    📝 Editing repo 'rust-lang/crates.io':
      Rulesets:
          Updating 'main'
            Bypass Actors: [] => [RulesetBypassActor { actor_id: 2201425, actor_type: Integration, bypass_mode: Always }]
            Restrict updates: true
    📝 Editing repo 'rust-lang/rustfmt':
      Rulesets:
          Updating 'main'
            Restrict updates: true
          Updating 'libsyntax'
            Restrict updates: true
          Updating 'rust-1.*'
            Restrict updates: true

[rustbot]

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.

[marcoieni]

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.

The file was modified in a retro compatible way (I added a method to a struct) ✅

[Kobzol]

Rulesets are complicated enough on their own, but having to think about supporting branch protections and rulesets at the same time is something else =D I assume that prevent-update controls both whether you can push to a branch and whether you can approve a PR that targets that branch (but I haven't checked that).

Also for bypass actors, even if a PR is required, they don't actually have to create a PR, right? This is complicated.. :)

So:

• PR required = false + prevent-updates = false means that anyone can push to a branch. Probably not very common, but can still be useful, as you might still want to prevent deleting a branch.
• PR required = true + prevent-updates = false means that you have to open a PR, and anyone with write access can then merge it.
• PR required = false + prevent-updates = true means that a PR is not required, but only bypass actors can push to a branch. This is the use-case for bors.
• PR required = true + prevent-updates = true means that you have to open a PR, and only bypass actors can merge that PR. But since bypass actors can actually override PR required = true, this setting is pretty much the same as the previous setting, I think? It seems that crates.io uses this one, so that bypass actors can actually push to the branch even without opening a PR, I think?

[Kobzol]

Based on what I wrote above, I think that if we want to support both pr-required and prevent-update, and we have pr-required = true as the default, then in the end prevent-update should default to false, otherwise it would mean that people with write access won't be able to merge PRs targeting that branch?

Edit: yeah, I tested it and this is how it works.