Skip to content

Commit

Permalink
Add support for custom roles in website, beyond "Team leader"
Browse files Browse the repository at this point in the history
  • Loading branch information
dtolnay committed Jan 2, 2024
1 parent 89bb40d commit c0d1e5f
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 6 deletions.
12 changes: 12 additions & 0 deletions docs/toml-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ zulip-stream = "t-lang"
# Default is 0.
weight = -100

# Customized roles held by a subset of the team's members, beyond "Team leader"
# which is rendered automatically for members of the `leads` array.
[[website.roles]]
# Kebab-case id for the role. This serves as a key for translations.
id = "cohost"
# Text to appear on the website beneath the team member's name and GitHub handle.
description = "Co-host"
# Subset of the team's members holding this role. A member may hold arbitrarily
# many roles. Each team member's roles will appear comma-separated in the same
# order as roles are listed in the website.roles array.
members = ["Crab01", "Crab03"]

# Define the mailing lists used by the team
# It's optional, and there can be more than one
[[lists]]
Expand Down
2 changes: 2 additions & 0 deletions rust_team_data/src/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ pub struct TeamMember {
pub github: String,
pub github_id: usize,
pub is_lead: bool,
#[serde(skip_serializing_if = "Vec::is_empty")]
pub roles: Vec<String>,
}

#[derive(Debug, Clone, Serialize, Deserialize)]
Expand Down
10 changes: 9 additions & 1 deletion src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ use schema::{Email, Team, TeamKind};

use anyhow::{bail, format_err, Error};
use log::{error, info, warn};
use std::{collections::HashMap, path::PathBuf};
use std::collections::{BTreeMap, HashMap};
use std::path::PathBuf;
use structopt::StructOpt;

#[derive(structopt::StructOpt)]
Expand Down Expand Up @@ -382,6 +383,7 @@ fn run() -> Result<(), Error> {
);
let mut teams: Vec<_> = data.teams().collect();
teams.sort_by_key(|team| team.name());
let mut roles = BTreeMap::new();
for team in teams {
if let Some(website) = team.website_data() {
let name = team.name();
Expand All @@ -391,8 +393,14 @@ fn run() -> Result<(), Error> {
name,
website.description()
);
for role in website.roles() {
roles.insert(&role.id, &role.description);
}
}
}
for (role_id, description) in roles {
println!("governance-role-{role_id} = {description}");
}
}
Cli::DumpPermission { ref name } => {
if !crate::schema::Permissions::available(data.config()).contains(name) {
Expand Down
14 changes: 14 additions & 0 deletions src/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,8 @@ pub(crate) struct WebsiteData {
name: String,
description: String,
page: Option<String>,
#[serde(default)]
roles: Vec<WebsiteRole>,
email: Option<String>,
repo: Option<String>,
discord_invite: Option<String>,
Expand All @@ -577,6 +579,10 @@ impl WebsiteData {
self.page.as_deref()
}

pub(crate) fn roles(&self) -> &[WebsiteRole] {
&self.roles
}

pub(crate) fn email(&self) -> Option<&str> {
self.email.as_deref()
}
Expand All @@ -601,6 +607,14 @@ impl WebsiteData {
}
}

#[derive(serde_derive::Deserialize, Debug)]
#[serde(deny_unknown_fields)]
pub(crate) struct WebsiteRole {
pub id: String,
pub description: String,
pub members: Vec<String>,
}

#[derive(serde_derive::Deserialize, Debug)]
#[serde(rename_all = "kebab-case", deny_unknown_fields)]
pub(crate) struct TeamList {
Expand Down
22 changes: 20 additions & 2 deletions src/static_api.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
use crate::data::Data;
use crate::schema::{Bot, Email, Permissions, RepoPermission, TeamKind, ZulipGroupMember};
use crate::schema::{
Bot, Email, Permissions, RepoPermission, TeamKind, WebsiteData, ZulipGroupMember,
};
use anyhow::Error;
use indexmap::IndexMap;
use log::info;
use rust_team_data::v1;
use std::collections::HashMap;
use std::path::Path;

pub(crate) struct Generator<'a> {
Expand Down Expand Up @@ -112,6 +115,17 @@ impl<'a> Generator<'a> {
let mut teams = IndexMap::new();

for team in self.data.teams() {
let website_data = team.website_data();
let mut website_roles = HashMap::new();
for role in website_data.map(WebsiteData::roles).unwrap_or(&[]) {
for assignee in &role.members {
website_roles
.entry(assignee.as_str())
.or_insert_with(Vec::new)
.push(role.id.clone());
}
}

let leads = team.leads();
let mut members = Vec::new();
for github_name in &team.members(self.data)? {
Expand All @@ -121,6 +135,9 @@ impl<'a> Generator<'a> {
github: (*github_name).into(),
github_id: person.github_id(),
is_lead: leads.contains(github_name),
roles: website_roles
.get(*github_name)
.map_or_else(Vec::new, Vec::clone),
});
}
}
Expand All @@ -135,6 +152,7 @@ impl<'a> Generator<'a> {
github: github_name.to_string(),
github_id: person.github_id(),
is_lead: false,
roles: Vec::new(),
});
}
}
Expand Down Expand Up @@ -167,7 +185,7 @@ impl<'a> Generator<'a> {
.collect::<Vec<_>>(),
})
.filter(|gh| !gh.teams.is_empty()),
website_data: team.website_data().map(|ws| v1::TeamWebsite {
website_data: website_data.map(|ws| v1::TeamWebsite {
name: ws.name().into(),
description: ws.description().into(),
page: ws.page().unwrap_or_else(|| team.name()).into(),
Expand Down
66 changes: 63 additions & 3 deletions src/validate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use crate::zulip::ZulipApi;
use anyhow::{bail, Error};
use log::{error, warn};
use regex::Regex;
use std::collections::{HashMap, HashSet};
use std::collections::hash_map::{Entry, HashMap};
use std::collections::HashSet;

macro_rules! checks {
($($f:ident,)*) => {
Expand Down Expand Up @@ -44,6 +45,7 @@ static CHECKS: &[Check<fn(&Data, &mut Vec<String>)>] = checks![
validate_zulip_group_ids,
validate_zulip_group_extra_people,
validate_repos,
validate_website_roles,
];

#[allow(clippy::type_complexity)]
Expand Down Expand Up @@ -538,9 +540,9 @@ fn validate_rfcbot_exclude_members(data: &Data, errors: &mut Vec<String>) {
/// Ensure team names are alphanumeric + `-`
fn validate_team_names(data: &Data, errors: &mut Vec<String>) {
wrapper(data.teams(), errors, |team, _| {
if !team.name().chars().all(|c| c.is_alphanumeric() || c == '-') {
if !ascii_kebab_case(team.name()) {
bail!(
"team name `{}` can only be alphanumeric with dashes",
"team name `{}` can only be alphanumeric with hyphens",
team.name()
);
}
Expand Down Expand Up @@ -774,6 +776,64 @@ fn validate_repos(data: &Data, errors: &mut Vec<String>) {
});
}

/// Enforce that website roles are only assigned to a valid team member, and
/// that the same role id always has a consistent description across teams
/// (because the role id becomes the Fluent id used for translation).
fn validate_website_roles(data: &Data, errors: &mut Vec<String>) {
let mut role_descriptions = HashMap::new();

wrapper(
data.teams().chain(data.archived_teams()),
errors,
|team, errors| {
let team_name = team.name();
if let Some(website_data) = team.website_data() {
let team_members = team.members(data)?;

for role in website_data.roles() {
let role_id = &role.id;
if !ascii_kebab_case(role_id) {
errors.push(format!(
"role id {role_id:?} must be alphanumeric with hyphens",
));
}

for assignee in &role.members {
if !team_members.contains(assignee.as_str()) {
errors.push(format!(
"person '{assignee}' with role '{role_id}' is not a \
member of team '{team_name}'",
));
}
}

match role_descriptions.entry(&role.id) {
Entry::Vacant(entry) => {
entry.insert(&role.description);
}
Entry::Occupied(entry) => {
if **entry.get() != role.description {
errors.push(format!(
"website role '{role_id}' has inconsistent description \
between different teams; if this is intentional, you \
must give those roles different ids",
));
}
}
}
}
}
Ok(())
},
);
}

/// We use Fluent ids which are lowercase alphanumeric with hyphens.
fn ascii_kebab_case(s: &str) -> bool {
s.chars()
.all(|c| c.is_ascii_lowercase() || c.is_ascii_digit() || c == '-')
}

fn wrapper<T, I, F>(iter: I, errors: &mut Vec<String>, mut func: F)
where
I: Iterator<Item = T>,
Expand Down
5 changes: 5 additions & 0 deletions teams/spec.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ description = "Creating and maintaining the specification for the Rust language"
zulip-stream = "t-spec"
repo = "https://github.com/rust-lang/spec"

[[website.roles]]
id = "spec-editor"
description = "Editor"
members = ["JoelMarcey"]

[rfcbot]
label = "T-spec"
name = "Spec"
Expand Down

0 comments on commit c0d1e5f

Please sign in to comment.