search

rust-lang / team

Rust teams structure
Apache License 2.0
312 stars 287 forks source link

sync zulip stream membership #1604

Open davidtwco opened 2 weeks ago

davidtwco commented 2 weeks ago

See rust-lang/sync-team#92 for the other half of this.

t-compiler want to use our private Zulip stream for some communications but we've held off on doing this as the membership of that stream is manually managed, that is inconvenient and we sometimes forget. In testing this patch, I found the stream membership was still out-of-sync!

I've tested this locally with the sync-team changes locally and absent the untested parts noted in that PR's description, it all works.

I've deliberately avoided adding an exhaustive schema for the streams, just their name as that's all that is necessary to adjust the membership, and that's all that the compiler team needs.

There's a reasonable amount of duplication with the types and functions for Zulip user groups, but we'll likely extend these types to manage more about streams, I think that's okay. Everything I've added should be reusable and compatible with representing streams fully, but I'll leave that for other patches like rust-lang/team#1244.

ehuss commented 2 weeks ago

Just so I'm clear, is the intention that this is only for private streams? I'm a little unclear, but it looks like it removes people if they are not explicitly listed in the team database, is that correct?

Can you add some docs to https://github.com/rust-lang/team/blob/master/docs/toml-schema.md documenting the new TOML structure?

davidtwco commented 1 week ago

Just so I'm clear, is the intention that this is only for private streams? I'm a little unclear, but it looks like it removes people if they are not explicitly listed in the team database, is that correct?

Can you add some docs to https://github.com/rust-lang/team/blob/master/docs/toml-schema.md documenting the new TOML structure?

Yeah, this would primarily be used for private streams.

davidtwco commented 1 week ago

Can you add some docs to https://github.com/rust-lang/team/blob/master/docs/toml-schema.md documenting the new TOML structure?

Added

MarcoIeni commented 1 week ago

The static_api test is failing in CI.

Also we could use flatten to get rid of the duplicated code 👍

davidtwco commented 1 week ago

Fixed the test and removed a bunch of the duplicated code

MarcoIeni commented 6 days ago

Overall looks good to me! I just left a few minor comments 👍