Merge pull request #1032 from rylev/allow-extending-teams

Allow extending teams from other teams

docs/toml-schema.md

@@ -55,6 +55,9 @@ members = [
 alumni = [
     "buildbot",
 ]
+# Optional, name of other teams whose members will be included as members of this team.
+# Defaults to empty.
+included-teams = []
 # Optional, include all members of all other teams.
 # Defaults to false.
 include-all-team-members = false

src/data.rs

@@ -138,12 +138,12 @@ impl Data {
         self.people.values()
     }
 
-    pub(crate) fn active_members(&self) -> HashSet<&str> {
-        self.teams
-            .values()
-            .filter(|team| !team.is_alumni_team())
-            .flat_map(|team| team.members(self))
-            .collect()
+    pub(crate) fn active_members(&self) -> Result<HashSet<&str>, Error> {
+        let mut active = HashSet::new();
+        for team in self.teams.values().filter(|team| !team.is_alumni_team()) {
+            active.extend(team.members(self)?)
+        }
+        Ok(active)
     }
 
     pub(crate) fn repos(&self) -> impl Iterator<Item = &Repo> {

src/main.rs

@@ -280,8 +280,12 @@ fn run() -> Result<(), Error> {
             println!("teams:");
             let mut teams: Vec<_> = data
                 .teams()
-                .filter(|team| team.members(&data).contains(person.github()))
-                .collect();
+                .filter_map(|team| match team.contains_person(&data, person) {
+                    Ok(true) => Some(Ok(team)),
+                    Ok(false) => None,
+                    Err(e) => Some(Err(e)),
+                })
+                .collect::<Result<_, _>>()?;
             teams.sort_by_key(|team| team.name());
             if teams.is_empty() {
                 println!("  (none)");
@@ -395,7 +399,7 @@ fn run() -> Result<(), Error> {
             if !crate::schema::Permissions::available(data.config()).contains(name) {
                 failure::bail!("unknown permission: {}", name);
             }
-            let mut allowed = crate::permissions::allowed_people(&data, name)
+            let mut allowed = crate::permissions::allowed_people(&data, name)?
                 .into_iter()
                 .map(|person| person.github())
                 .collect::<Vec<_>>();
@@ -441,7 +445,7 @@ fn dump_team_members(
     tab_offset: u8,
 ) -> Result<(), Error> {
     let leads = team.leads();
-    let mut members = team.members(data).into_iter().collect::<Vec<_>>();
+    let mut members = team.members(data)?.into_iter().collect::<Vec<_>>();
     members.sort_unstable();
     for member in members {
         if only_leads && !leads.contains(member) {

src/permissions.rs

@@ -119,17 +119,21 @@ impl Permissions {
     }
 }
 
-pub(crate) fn allowed_people<'a>(data: &'a Data, permission: &str) -> Vec<&'a Person> {
+pub(crate) fn allowed_people<'a>(
+    data: &'a Data,
+    permission: &str,
+) -> Result<Vec<&'a Person>, Error> {
     let mut members_with_perms = HashSet::new();
     for team in data.teams() {
         if team.permissions().has(permission) {
-            members_with_perms.extend(team.members(data));
+            members_with_perms.extend(team.members(data)?);
         }
         if team.leads_permissions().has(permission) {
             members_with_perms.extend(team.leads());
         }
     }
-    data.people()
+    Ok(data
+        .people()
         .filter(|p| members_with_perms.contains(p.github()) || p.permissions().has(permission))
-        .collect()
+        .collect())
 }

src/schema.rs

@@ -220,9 +220,19 @@ impl Team {
         self.discord_roles.as_ref()
     }
 
-    pub(crate) fn members<'a>(&'a self, data: &'a Data) -> HashSet<&'a str> {
+    pub(crate) fn members<'a>(&'a self, data: &'a Data) -> Result<HashSet<&'a str>, Error> {
         let mut members: HashSet<_> = self.people.members.iter().map(|s| s.as_str()).collect();
 
+        for team in &self.people.included_teams {
+            let team = data.team(team).ok_or_else(|| {
+                err_msg(format!(
+                    "team '{}' includes members from non-existent team '{}'",
+                    self.name(),
+                    team
+                ))
+            })?;
+            members.extend(team.members(data)?);
+        }
         let mut include_leads = |kind| {
             for team in data.teams() {
                 if team.name != self.name && team.kind == kind {
@@ -251,26 +261,29 @@ impl Team {
                 {
                     continue;
                 }
-                members.extend(team.members(data));
+                members.extend(team.members(data)?);
             }
         }
         if self.is_alumni_team() {
-            let active_members = data.active_members();
+            let active_members = data.active_members()?;
             let alumni = data
                 .teams()
                 .chain(data.archived_teams())
                 .flat_map(|t| t.alumni())
                 .map(|a| a.as_str());
-            let members_of_archived_teams =
-                data.archived_teams().flat_map(|team| team.members(data));
+            let mut members_of_archived_teams = HashSet::new();
+
+            for t in data.archived_teams() {
+                members_of_archived_teams.extend(t.members(data)?);
+            }
 
             members.extend(
                 alumni
                     .chain(members_of_archived_teams)
                     .filter(|person| !active_members.contains(person)),
             )
         }
-        members
+        Ok(members)
     }
 
     pub(crate) fn alumni(&self) -> &[String] {
@@ -290,13 +303,13 @@ impl Team {
             };
 
             let mut members = if raw_list.include_team_members {
-                self.members(data)
+                self.members(data)?
             } else {
                 HashSet::new()
             };
             if raw_list.include_subteam_members {
                 for subteam in data.subteams_of(&self.name) {
-                    members.extend(subteam.members(data));
+                    members.extend(subteam.members(data)?);
                 }
             }
             for person in &raw_list.extra_people {
@@ -306,7 +319,7 @@ impl Team {
                 let team = data
                     .team(team)
                     .ok_or_else(|| err_msg(format!("team {} is missing", team)))?;
-                members.extend(team.members(data));
+                members.extend(team.members(data)?);
             }
 
             for member in members.iter() {
@@ -341,7 +354,7 @@ impl Team {
             };
 
             let mut members = if raw_group.include_team_members {
-                self.members(data)
+                self.members(data)?
             } else {
                 HashSet::new()
             };
@@ -352,7 +365,7 @@ impl Team {
                 let team = data
                     .team(team)
                     .ok_or_else(|| err_msg(format!("team {} is missing", team)))?;
-                members.extend(team.members(data));
+                members.extend(team.members(data)?);
             }
             for excluded in &raw_group.excluded_people {
                 if !members.remove(excluded.as_str()) {
@@ -390,15 +403,15 @@ impl Team {
         let mut result = Vec::new();
         for github in &self.github {
             let mut members = self
-                .members(data)
+                .members(data)?
                 .iter()
                 .filter_map(|name| data.person(name).map(|p| (p.github(), p.github_id())))
                 .collect::<Vec<_>>();
             for team in &github.extra_teams {
                 members.extend(
                     data.team(team)
                         .ok_or_else(|| failure::err_msg(format!("missing team {}", team)))?
-                        .members(data)
+                        .members(data)?
                         .iter()
                         .filter_map(|name| data.person(name).map(|p| (p.github(), p.github_id()))),
                 );
@@ -419,7 +432,7 @@ impl Team {
 
     pub(crate) fn discord_ids(&self, data: &Data) -> Result<Vec<usize>, Error> {
         Ok(self
-            .members(data)
+            .members(data)?
             .iter()
             .flat_map(|name| data.person(name).map(|p| p.discord_id()))
             .flatten()
@@ -434,6 +447,11 @@ impl Team {
     pub(crate) fn explicit_members(&self) -> &Vec<String> {
         &self.people.members
     }
+
+    pub(crate) fn contains_person(&self, data: &Data, person: &Person) -> Result<bool, Error> {
+        let members = self.members(data)?;
+        Ok(members.contains(person.github()))
+    }
 }
 
 #[derive(serde_derive::Deserialize, Debug)]
@@ -484,6 +502,8 @@ struct TeamPeople {
     members: Vec<String>,
     #[serde(default)]
     alumni: Vec<String>,
+    #[serde(default)]
+    included_teams: Vec<String>,
     #[serde(default = "default_false")]
     include_team_leads: bool,
     #[serde(default = "default_false")]

src/static_api.rs

@@ -114,7 +114,7 @@ impl<'a> Generator<'a> {
         for team in self.data.teams() {
             let leads = team.leads();
             let mut members = Vec::new();
-            for github_name in &team.members(self.data) {
+            for github_name in &team.members(self.data)? {
                 if let Some(person) = self.data.person(github_name) {
                     members.push(v1::TeamMember {
                         name: person.name().into(),
@@ -257,7 +257,7 @@ impl<'a> Generator<'a> {
 
     fn generate_permissions(&self) -> Result<(), Error> {
         for perm in &Permissions::available(self.data.config()) {
-            let allowed = crate::permissions::allowed_people(self.data, perm);
+            let allowed = crate::permissions::allowed_people(self.data, perm)?;
             let mut github_users = allowed
                 .iter()
                 .map(|p| p.github().to_string())
@@ -290,7 +290,7 @@ impl<'a> Generator<'a> {
         for team in self.data.teams() {
             if let Some(rfcbot) = team.rfcbot_data() {
                 let mut members = team
-                    .members(self.data)
+                    .members(self.data)?
                     .into_iter()
                     .map(|s| s.to_string())
                     .filter(|member| !rfcbot.exclude_members.contains(member))

src/validate.rs

@@ -120,7 +120,15 @@ pub(crate) fn validate(data: &Data, strict: bool, skip: &[&str]) -> Result<(), E
 
 /// Ensure working group names start with `wg-`
 fn validate_name_prefixes(data: &Data, errors: &mut Vec<String>) {
-    fn ensure_prefix(team: &Team, kind: TeamKind, prefix: &str) -> Result<(), Error> {
+    fn ensure_prefix(
+        team: &Team,
+        kind: TeamKind,
+        prefix: &str,
+        exceptions: &[&str],
+    ) -> Result<(), Error> {
+        if exceptions.contains(&team.name()) {
+            return Ok(());
+        }
         if team.kind() == kind && !team.name().starts_with(prefix) {
             bail!(
                 "{} `{}`'s name doesn't start with `{}`",
@@ -140,8 +148,13 @@ fn validate_name_prefixes(data: &Data, errors: &mut Vec<String>) {
         Ok(())
     }
     wrapper(data.teams(), errors, |team, _| {
-        ensure_prefix(team, TeamKind::WorkingGroup, "wg-")?;
-        ensure_prefix(team, TeamKind::ProjectGroup, "project-")?;
+        ensure_prefix(team, TeamKind::WorkingGroup, "wg-", &["wg-leads"])?;
+        ensure_prefix(
+            team,
+            TeamKind::ProjectGroup,
+            "project-",
+            &["project-group-leads"],
+        )?;
         Ok(())
     });
 }
@@ -175,7 +188,7 @@ fn validate_subteam_of(data: &Data, errors: &mut Vec<String>) {
 /// Ensure team leaders are part of the teams they lead
 fn validate_team_leads(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, |team, errors| {
-        let members = team.members(data);
+        let members = team.members(data)?;
         wrapper(team.leads().iter(), errors, |lead, _| {
             if !members.contains(lead) {
                 bail!(
@@ -193,7 +206,7 @@ fn validate_team_leads(data: &Data, errors: &mut Vec<String>) {
 /// Ensure team members are people
 fn validate_team_members(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, |team, errors| {
-        wrapper(team.members(data).iter(), errors, |member, _| {
+        wrapper(team.members(data)?.iter(), errors, |member, _| {
             if data.person(member).is_none() {
                 bail!(
                     "person `{}` is member of team `{}` but doesn't exist",
@@ -209,11 +222,17 @@ fn validate_team_members(data: &Data, errors: &mut Vec<String>) {
 
 /// Ensure alumni are not active
 fn validate_alumni(data: &Data, errors: &mut Vec<String>) {
-    let active_members = data.active_members();
+    let active_members = match data.active_members() {
+        Ok(ms) => ms,
+        Err(e) => {
+            errors.push(e.to_string());
+            return;
+        }
+    };
     wrapper(data.team("alumni").iter(), errors, |alumni_team, errors| {
         let mut explicit_members: HashSet<_> = alumni_team.explicit_members().iter().collect();
         // Ensure alumni team members are not active
-        wrapper(alumni_team.members(data).iter(), errors, |member, _| {
+        wrapper(alumni_team.members(data)?.iter(), errors, |member, _| {
             if active_members.contains(member) {
                 bail!("alumni team includes active member '{}'", member)
             }
@@ -243,7 +262,7 @@ fn validate_inactive_members(data: &Data, errors: &mut Vec<String>) {
         data.teams().chain(data.archived_teams()),
         errors,
         |team, _| {
-            let members = team.members(data);
+            let members = team.members(data)?;
             for member in members {
                 referenced_members.insert(member);
             }
@@ -287,7 +306,7 @@ fn validate_list_email_addresses(data: &Data, errors: &mut Vec<String>) {
         if team.lists(data)?.is_empty() {
             return Ok(());
         }
-        wrapper(team.members(data).iter(), errors, |member, _| {
+        wrapper(team.members(data)?.iter(), errors, |member, _| {
             if let Some(member) = data.person(member) {
                 if let Email::Missing = member.email() {
                     bail!(
@@ -374,7 +393,7 @@ fn validate_people_addresses(data: &Data, errors: &mut Vec<String>) {
 /// Ensure members of teams with permissions don't explicitly have those permissions
 fn validate_duplicate_permissions(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, |team, errors| {
-        wrapper(team.members(data).iter(), errors, |member, _| {
+        wrapper(team.members(data)?.iter(), errors, |member, _| {
             if let Some(person) = data.person(member) {
                 for permission in &Permissions::available(data.config()) {
                     if team.permissions().has(permission)
@@ -431,7 +450,7 @@ fn validate_rfcbot_exclude_members(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, move |team, errors| {
         if let Some(rfcbot) = team.rfcbot_data() {
             let mut exclude = HashSet::new();
-            let members = team.members(data);
+            let members = team.members(data)?;
             wrapper(rfcbot.exclude_members.iter(), errors, move |member, _| {
                 if !exclude.insert(member) {
                     bail!(
@@ -548,7 +567,7 @@ fn validate_project_groups_have_parent_teams(data: &Data, errors: &mut Vec<Strin
 fn validate_discord_team_members_have_discord_ids(data: &Data, errors: &mut Vec<String>) {
     wrapper(data.teams(), errors, |team, _| {
         if team.discord_roles().is_some() && team.name() != "all" {
-            let team_members = team.members(data);
+            let team_members = team.members(data)?;
             if team_members.len() != team.discord_ids(data)?.len() {
                 let members: String = team_members
                     .into_iter()
@@ -620,7 +639,7 @@ fn validate_zulip_group_ids(data: &Data, errors: &mut Vec<String>) {
         if groups.is_empty() || groups.iter().all(|g| !g.includes_team_members()) {
             return Ok(());
         }
-        wrapper(team.members(data).iter(), errors, |member, _| {
+        wrapper(team.members(data)?.iter(), errors, |member, _| {
             if let Some(member) = data.person(member) {
                 if member.zulip_id().is_none() {
                     bail!(

teams/inside-rust-reviewers.toml

@@ -4,8 +4,7 @@ kind = "marker-team"
 [people]
 leads = []
 members = []
-include-team-leads = true
-include-project-group-leads = true
+included-teams = ["leadership-council", "leads", "project-group-leads"]
 
 [[github]]
 orgs = ["rust-lang"]