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

CODEOWNERS: remove domenkozar, add docs team #680

Merged
merged 1 commit into from
Aug 21, 2023

Conversation

asymmetric
Copy link
Contributor

@asymmetric asymmetric commented Aug 18, 2023

The reasoning is:

  • domenkozar has transferred ownership of the infrastructure Transfer ownership of infrastructure #481
  • unless someone is a team lead on a specific sub-team (as is the case for zmitchell), we (people with write access to nix.dev) should all be able to merge PRs

@cloudflare-pages
Copy link

cloudflare-pages bot commented Aug 18, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: b767ac2
Status: ✅  Deploy successful!
Preview URL: https://dc2e8794.nix-dot-dev.pages.dev
Branch Preview URL: https://reset-codeowners.nix-dot-dev.pages.dev

View logs

@infinisil
Copy link
Member

  • we (people with write access to nix.dev) should all be able to merge PRs

GitHub's code owners feature doesn't have an influence on who can merge PR's unless you enable it (which is not the case I'm pretty sure). So it's really just an easy way to automatically request reviews from specific people when certain files change.

This can be thought of a mechanism to signal who's responsible for which part of the code, and as such it would be great to always have at least one code owner for every file.

@proofconstruction
Copy link
Contributor

On the changes:

  • I agree with removal of domenkozar from CODEOWNERS to close Transfer ownership of infrastructure #481.

  • fricklerhandwerk is still the documentation team lead and should remain in CODEOWNERS unless that changes.

In general:

The CODEOWNERS file should reflect what infinisil noted above (emphasis mine):

This can be thought of a mechanism to signal who's responsible for which part of the code, and as such it would be great to always have at least one code owner for every file.

Since we (on the docs team specifically, but also anyone involved in some way in the broader NixOS project) still operate as a praxicracy, presence in the CODEOWNERS file should reflect individuals' chosen responsibility to contribute in this way; we are here for as long as we want to be, participating at the level we choose/are able to, and this file reifies that commitment.

It should be reviewed periodically (at minimum once per year but maybe every 2-3 months) for accuracy as compared to the running make-up of the team at that time, at least to avoid spamming people with review requests if they don't have the time/interest to do so.

@asymmetric
Copy link
Contributor Author

asymmetric commented Aug 18, 2023

@infinisil oops you're right, this doesn't currently block merges (I could even self-merge this PR).

So it's really just an easy way to automatically request reviews from specific people when certain files change.

OK, but then should we really automatically ask reviews of @fricklerhandwerk and @domenkozar for everything under source/? I mean, they probably added themselves, so 🤷

This can be thought of a mechanism to signal who's responsible for which part of the code

@proofconstruction Agreed. So who's responsible for what? In my mind, it's pretty clear-cut with @zmitchell. As for the rest, I'm not so sure.

@proofconstruction
Copy link
Contributor

Now I'm thinking that Domen should remain in CODEOWNERS in those places if he chooses to continue being responsible for those things, and we should consider just leaving the file as-is, close this PR, re-close the infrastructure ownership one, and then open a separate Issue to discuss this further so everyone interested can have a say over a longer period (maybe 2 weeks?).

@proofconstruction Agreed. So who's responsible for what? In my mind, it's pretty clear-cut with @zmitchell. As for the rest, I'm not so sure.

I don't know either. Nobody has edited the file to add themselves and take ownership of anything. Part of that is probably a broader lack of a concrete plan, or at least lack of focus on this plan (which may already be complete as-written; we should discuss the evaluation & final report soon), and partly a lack of resources (funding, time, energy, attention, etc) on the part of individuals. Speaking for myself, I'd be open to taking ownership of some pieces but I don't know which ones (they may not exist yet) and I haven't been very good so far about taking care of the things I have said I'd do (attention is my scarcest resource).

Copy link
Member

@infinisil infinisil left a comment

Choose a reason for hiding this comment

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

Discussed this and pushed changes during the docs team meeting today, let's have the entire docs team be added as code owner, we can change this to be more specific later.

The reasoning is:
- domenkozar has transferred ownership of the infrastructure #481
- All the doc team members should get pinged for changes, let's see how
  it goes for some time, we can make it more specific later

Co-Authored-By: @infinisil, @fricklerhandwerk, @proofconstruction
@fricklerhandwerk fricklerhandwerk merged commit 00eab32 into master Aug 21, 2023
3 checks passed
@infinisil infinisil deleted the reset-codeowners branch August 21, 2023 16:58
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-08-21-documentation-team-meeting-notes-74/32083/1

@infinisil infinisil changed the title CODEOWNERS: remove domenkozar, fricklerhandwerk CODEOWNERS: remove domenkozar, add docs team Aug 23, 2023
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

5 participants