Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for custom roles in website, beyond "Team leader" #1154

Merged
merged 6 commits into from
Jan 8, 2024

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Dec 22, 2023

I would like to render "Editor" below @JoelMarcey's entry in https://www.rust-lang.org/governance/teams/lang#spec, in the same location that others have "Team leader".

Source: https://blog.rust-lang.org/inside-rust/2023/11/15/spec-vision.html#specification-team

Output of cargo run -- dump-website for translations:

...
governance-team-wg-triage-name = Triage working group
governance-team-wg-triage-description = Triaging repositories under the rust-lang organisation

governance-team-wg-wasm-name = WebAssembly (WASM) working group
governance-team-wg-wasm-description = Improving on the end-to-end experience of embedding Rust code in JS libraries and apps via WebAssembly

governance-role-spec-editor = Editor

Output of cargo run -- static-api under v1/teams/spec.json:

{
  "name": "spec",
  "kind": "team",
  "subteam_of": "lang",
  "members": [
    {
      "name": "Mara Bos",
      "github": "m-ou-se",
      "github_id": 783247,
      "is_lead": true
    },
    {
      "name": "Felix Klock",
      "github": "pnkfelix",
      "github_id": 173127,
      "is_lead": true
    },
    {
      "name": "Eric Huss",
      "github": "ehuss",
      "github_id": 43198,
      "is_lead": false
    },
    {
      "name": "Joel Marcey",
      "github": "JoelMarcey",
      "github_id": 3757713,
      "is_lead": false,
      "roles": [
        "spec-editor"
      ]
    }
  ],

@dtolnay
Copy link
Member Author

dtolnay commented Dec 22, 2023

Another possible application: team affiliation in https://www.rust-lang.org/governance/teams/leadership-council#leadership-council. "Lang team rep"

rylev
rylev previously approved these changes Jan 3, 2024
Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this! Overall it's a great change, and I'd be happy to merge except for one thing.

The current schema requires duplicating the member's name both in the team's members array as well as part of role's members array. Would it make sense to see if we can change the team members array schema to allow for specifying roles inline to the member declaration?

Perhaps something like:

members = [
  "rylev",
  { name =  "dtolnay", role = "foo" }
]

According to the toml spec this should be fine, but I think maybe some toml parsers struggle with this syntax?

src/static_api.rs Outdated Show resolved Hide resolved
src/validate.rs Outdated Show resolved Hide resolved
@dtolnay
Copy link
Member Author

dtolnay commented Jan 4, 2024

Updated to set roles inline in the team's existing members array, as you suggested.

Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I think we can merge as is as the static API looks good (and that's the only thing that is somewhat tricky to change after the fact). However, I'd like to get your opinion on my comments on the schema.

src/schema.rs Outdated Show resolved Hide resolved
docs/toml-schema.md Outdated Show resolved Hide resolved
docs/toml-schema.md Outdated Show resolved Hide resolved
src/schema.rs Show resolved Hide resolved
rylev
rylev previously approved these changes Jan 8, 2024
Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a great start! Let's merge this and we can adjust from here.

@dtolnay seems like there might be conflicts that need to be resolved first though.

@dtolnay
Copy link
Member Author

dtolnay commented Jan 8, 2024

Rebased to fix superficial conflict with #1164 in src/validate.rs.

@rylev rylev merged commit a9ba0cb into rust-lang:master Jan 8, 2024
1 check passed
@dtolnay dtolnay deleted the websiteroles branch January 8, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants