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 ipfs-tech org to key mf repos #109

Merged
merged 6 commits into from
Oct 29, 2024

Conversation

bumblefudge
Copy link
Contributor

Summary

Hey there-- as I've been leading the recent IETF efforts for multiformats and getting more involved in the general upkeep of docs and such, I never asked for privileges here, but Rod asked me to do so and the timing seems good since the IPFS Foundation now officially exists. I'm asking for privileges for the Foundation org rather than for me personally just to simplify delegation and maintenance down the road.

Why do you need this?

Just want to include multiformats/website in periodic documentation refreshes and have push-rights in multicodec if the IETF WG spins up in coming months.

What else do we need to know?

DRI: myself, @mishmosh, @darobin (current members of the ipfs-tech org)

Reviewer's Checklist

  • It is clear where the request is coming from (if unsure, ask)
  • All the automated checks passed
  • The YAML changes reflect the summary of the request
  • The Terraform plan posted as a comment reflects the summary of the request

@bumblefudge bumblefudge requested review from a team as code owners October 28, 2024 10:05
Copy link
Contributor

The following access changes will be introduced as a result of applying the plan:

Access Changes
There will be no access changes

@achingbrain
Copy link
Member

achingbrain commented Oct 28, 2024

Looking at the changes here:

  1. You've added an "ipfs-tech" team to various repos - this team is not defined in the teams section so I don't think this will do anything - you may need to also create it there
  2. The team has been granted pull access to public repos - maybe @galargh can correct me here but I think this is unnecessary as everyone has pull access to public repos?
  3. If the day-to-day tasks for the team are pushing branches and/or reviewing/merging PRs, just grant the team push access, no need for admin - if you need to do admin things, you can temporarily elevate your privileges with a further PR here, but for every day usage it's best to proceed on the basis of the principle of least privilege

@bumblefudge
Copy link
Contributor Author

Looking at the changes here:

  1. You've added an "ipfs-tech" team to various repos - this team is not defined in the teams section so I don't think this will do anything

whoops, I didn't realize the teams were defined within this document, I assumed they were pointers to orgs registered in the greater github identity system. fixed in commit below.

  1. The team has been granted pull access to public repos - maybe @galargh can correct me here but I think this is redundant as everyone has pull access to public repos?

whoops that was a typo, moved to push for the same repo in commit below.

  1. If the day-to-day tasks for the team are pushing branches and/or reviewing/merging PRs, just grant the team push access, no need for admin - if you need to do admin things, you can temporarily elevate your privileges with a further PR here, but for every day usage it's best to proceed on the basis of the principle of least privilege

I defer to @mishmosh on how best to handle this, honestly-- I figured it was the Foundation's remit to make sure there are active maintainers for all the repos here listed, and thus being able to name them is probably good, but my personal remit and medium-term goals can all be met by just giving A.) me (or B.) the whole org) push rights. Should I open a separate PR for A or B and leave this one open for discussion?

@rvagg
Copy link
Member

rvagg commented Oct 28, 2024

I think the preference is supposed to be for just enough access as required, then elevation on an as-needed basis. So prefer to not use admin unless there's a good reason for it (there typically isn't since many normal admin tasks can be done thorough this repo).

Copy link
Contributor

Before merge, verify that all the following plans are correct. They will be applied as-is after the merge.

Terraform plans

multiformats

Terraform used the selected providers to generate the following execution
plan. Resource actions are indicated with the following symbols:
  + create

Terraform will perform the following actions:

  # github_team.this["ipfs-tech"] will be created
  + resource "github_team" "this" {
      + create_default_maintainer = false
      + description               = "IPFS Foundation spec and registry maintenance crew"
      + etag                      = (known after apply)
      + id                        = (known after apply)
      + members_count             = (known after apply)
      + name                      = "ipfs-tech"
      + node_id                   = (known after apply)
      + privacy                   = "secret"
      + slug                      = (known after apply)
    }

  # github_team_membership.this["ipfs-tech:bumblefudge"] will be created
  + resource "github_team_membership" "this" {
      + etag     = (known after apply)
      + id       = (known after apply)
      + role     = "maintainer"
      + team_id  = (known after apply)
      + username = "bumblefudge"
    }

  # github_team_membership.this["ipfs-tech:darobin"] will be created
  + resource "github_team_membership" "this" {
      + etag     = (known after apply)
      + id       = (known after apply)
      + role     = "maintainer"
      + team_id  = (known after apply)
      + username = "darobin"
    }

  # github_team_membership.this["ipfs-tech:mishmosh"] will be created
  + resource "github_team_membership" "this" {
      + etag     = (known after apply)
      + id       = (known after apply)
      + role     = "maintainer"
      + team_id  = (known after apply)
      + username = "mishmosh"
    }

  # github_team_repository.this["ipfs-tech:multiaddr"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multiaddr"
      + team_id    = (known after apply)
    }

  # github_team_repository.this["ipfs-tech:multibase"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multibase"
      + team_id    = (known after apply)
    }

  # github_team_repository.this["ipfs-tech:multicodec"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multicodec"
      + team_id    = (known after apply)
    }

  # github_team_repository.this["ipfs-tech:multiformats"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "multiformats"
      + team_id    = (known after apply)
    }

  # github_team_repository.this["ipfs-tech:specs"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "push"
      + repository = "specs"
      + team_id    = (known after apply)
    }

  # github_team_repository.this["ipfs-tech:website"] will be created
  + resource "github_team_repository" "this" {
      + etag       = (known after apply)
      + id         = (known after apply)
      + permission = "admin"
      + repository = "website"
      + team_id    = (known after apply)
    }

Plan: 10 to add, 0 to change, 0 to destroy.

@rvagg rvagg merged commit 53b25a5 into multiformats:master Oct 29, 2024
6 checks passed
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.

3 participants