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

[RFC 0138] Developing RFCs in repositories #138

Closed
wants to merge 5 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 106 additions & 0 deletions rfcs/0138-rfc-repos.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
---
feature: rfc-repos
start-date: 2022-01-10
author: Silvan Mosberger
co-authors: (find a buddy later to help out with the RFC)
shepherd-team: @winterqt, @lheckemann
shepherd-leader: @lheckemann
related-issues: (will contain links to implementation PRs)
---

# Summary
[summary]: #summary

Develop and discuss RFCs in repositories instead of a pull request.

# Motivation
[motivation]: #motivation

RFC discussions are currently held in single pull requests on the [rfcs](https://github.com/NixOS/rfcs/pulls) repository.
- Due to GitHub's limitations on pull requests, this quickly becomes unmanageable.
The most prominent example of this is [the Flakes RFC](https://github.com/NixOS/rfcs/pull/49) with over 500 comments.
- These PR threads easily get polluted with minor points like typos and wording improvements,
which are not meaningful to the RFCs contents, but distract from other more important discussions.
- It's easiest to comment at the top-level, without threads,
especially when there's no relevant line of the diff to comment on for a thread.
This frequently causes multiple discussions to be interleaved at the top-level.

# Detailed design
[design]: #detailed-design

The RFC process is changed to use repositories to develop and review RFCs instead.
To create a new RFC, an issue on the [rfcs](https://github.com/NixOS/rfcs) repository is opened.
It should contain the following contents describing the process:

> This RFC is developed as the README.md of this repository: https://github.com/$owner/$repo
>
> To review the RFC
> - Open https://github.com/$owner/$repo/blob/master/README.md?plain=1
> - Select the lines you wish to comment on
> - Click the "..." menu on the left
> - Select "Reference in new issue" to open a new issue commenting on those lines
>
> To suggest changes, open a PR against the repository.
>
> All issues and pull requests must be resolved before the FCP can be initiated and completed.
>
> This issue may only be used for RFC meta-discussions, such as shepherd nominations, FCP periods, meeting schedules, etc.

The FCP must be initiated on a specific commit of the repository.
When the FCP passes, the repositories contents are committed to the [rfcs](https://github.com/NixOS/rfcs) repository by the [RFC steering committee](https://github.com/NixOS/rfcs#rfc-steering-committee) with a commit that closes the original issue.

## Workflow to get RFC updates
To get updates for RFC's, instead of subscribing to the PR, one has to watch the repository.

## Repository transfer

Once the RFC is merged, the repository has to be transferred to the NixOS organization under https://github.com/NixOS/rfc-NUMBER. This is to ensure the discussions aren't lost in the future. The repository will then be archived.
Copy link
Member

Choose a reason for hiding this comment

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

I think it should be done in a more centralized form.

This is merely an archive of discussions. Then a possible design is

  1. Create the repo https://github.com/NixOS/rfc-archives
  2. For each merged RFC:
    1. Create a branch https://github.com/NixOS/rfc-archives/tree/RFC-<number>
    2. Save (by using some git magic I don't remember perfectly now) the discussion
    3. Update MERGED RFCS section on README.MD (including a link to the branch created above)
    4. Profit!

Copy link
Member Author

Choose a reason for hiding this comment

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

2. Save (by using some git magic I don't remember perfectly now) the discussion

The discussion happens in GitHub issues and pull requests though, it's not attached to Git itself. And while issues can be moved to separate repositories, moving discussions from separate RFCs to the same repository would create a very messy issue list.


# Examples and Interactions
[examples-and-interactions]: #examples-and-interactions


# Drawbacks
[drawbacks]: #drawbacks
Copy link
Member

@gilice gilice Mar 12, 2023

Choose a reason for hiding this comment

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

  • the convenient, already existing URL scheme to find any RFC, eg. https://github.com/nixos/RFCs/pull/138,
    (s/138/RFC num), breaks.
  • Users that give one review don't automatically become watchers on the whole repo

Copy link
Member Author

Choose a reason for hiding this comment

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

  • the convenient, already existing URL scheme to find any RFC, eg. https://github.com/nixos/RFCs/pull/138,
    (s/138/RFC num), breaks.

This actually still works, because GitHub redirects ../pull/138 to ../issues/138 if it's not a pull request but an issue (which would be the case in the proposal here).

I added the other point to the Drawbacks.

Copy link
Member

Choose a reason for hiding this comment

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

  • For the current process there is some acceptance of separation between authorship and reviews (including edit suggestions). If suggestions become pull requests, the norms will have to be rebuilt. Among other things, it might become unclear who counts as a coauthor for the purpose of non-eligibility to be a shepherd.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks, I added this to the drawbacks!

Copy link
Member

@lheckemann lheckemann Apr 3, 2023

Choose a reason for hiding this comment

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

Shepherds already often end up making material contributions to the RFC text in the current process, as a fairly natural consequence of the process itself.

IMHO: this is okay and doesn't hurt the spirit of the RFC process -- it still ensures that it's not just an author and a coauthor making decisions amongst themselves without anyone else in the community noticing. I think that if a team of 5 people has co-written an RFC it's perfectly fine for 3 of them to form the shepherd team (though any nominations outside the shepherd team should probably be given extra consideration by the steering committee).

Copy link
Member

Choose a reason for hiding this comment

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

I noticed 2 unmentioned drawbacks (one of which I still don't remember), mentioning one of them here.

  1. Force pushes to edit something will not be noticed if done directly on the default branch.
  2. I'll edit it in once I remember, sorry!

As for 1, I have a potential solution in mind; we can transfer the RFC repository to the NixOS org as a WIP RFC and protect the default branch so that all changes are visible in PRs as usual. Also, I really loved the idea of pre-RFCs, I think pre-RFCs could integrate well with this where the transfer would only occur once the pre-RFC has matured into a "normal" one.

Copy link
Member

Choose a reason for hiding this comment

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

The 2nd drawback I couldn't remember in my previous thread:

If an RFC has a relatively large amount of traffic, multiple discussions can start on the same logical unit discussing more or less the same thing (or even the exact same thing). While this can be considered relatively trivial to manage, it's still an overhead that I thought needs to be mentioned (which apparently already was here: #138 (comment).

This is a consequence of the previously mentioned absence of the line-comment view.

Solution?

A potential way of solving this can be to have a great label architecture (much like nixpkgs). We could have "broad" labels which could be: drawbacks, alternatives, future-work; pretty self-explanatory but these would essentially mimic all the major headings in the RFC template. Unsure if a general rule can be applied to smaller labels, if not, they will just be a mess.

- There's no line-comment view anymore (the "Files changed" tab for PR's), where you can see the entire proposal and all (up-to-date) line comments.
- Commenting on an RFC won't automatically subscribe you to updates anymore.
- Less separation between RFC authors and reviewers with edit suggestions, making it harder to decide who may be a shepherd, since authors can't be shepherds.
- Force pushes won't be visible anymore in the history.
- Force pushing shouldn't be necessary for RFCs, since they always get squash merged anyways
- With the previous approach, perhaps the main use case of force pushing was to reduce the PR items to be loaded,
since commits also count towards the number of items loaded by GitHub.
This is obviously not necessary anymore.
- It would be good to protect the main branch for active RFCs,
which could be done by requiring the RFC repo be moved to the NixOS organisation.

# Alternatives
Copy link
Member

@piegamesde piegamesde Jan 10, 2023

Choose a reason for hiding this comment

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

I suggest putting more focus on Pre-RFCs and on discussion in the forum. Rust does this and it works really well IMO. A lot of the discussion noise in GitHub threads currently comes to RFCs being posted as drafts containing typos (I've also occasionally witnessed this on Matrix RFCs too), or more generally from the first impact of an idea in the author's head with the reality of a community's opinions. A lot of that can be absorbed by iterating outside of the formal RFC process. I think then the discussion workload on GitHub would remain manageable.

Even when keeping the approach of using GitHub repositories as suggested here, I'd still recommend putting more emphasis on pre-RFC discussions.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah that's a good suggestion. I think these two ideas go hand-in-hand: You can create a repository for development of the RFC beforehand, and if you want it to become more official, be looked at by more people, etc. open an issue in the rfcs repository linking to the repository where it's being developed.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe then a ful alternative could look like that:

  • in the RFC process: add a field «acknowledgements» to be used mainly for reviewers of early drafts who were only giving feedback and local suggestions and not co-writing the text; clarify these people are eligible to be shepherds
  • in the RFC process: establish that representing different viewpoints based on different use patterns in a shepherd team is desired, but only if different viewpoints are actually expressed during the early formal review
  • informally: encourage early direction-discussion on Discourse
  • informally: encourage splitting the concerns for making progress step by step
  • informally: if the minimal useful version is still rather large, encourage a round of drafting in a separate repository before submission, adding the active participants into early reviewers (well, possibly some will also become authors)

I do think that splitting small enough not to need a separate repo is good when possible, and I think that submitting for the final review via the old process after drafting in a repo might be still reasonable otherwise.

[alternatives]: #alternatives

## Optional instead of required

Instead of requiring this process, it could be opt-in for "bigger" RFCs.
- (-) It's not clear how to decide whether an RFC should have a repository, there's no way to know how big discussions become in advance.

## Fork branch instead of separate repository

Instead of creating a new repository for each RFC, a new branch in a fork can be created instead.
- (-) Means that a single GitHub user/organization can't have more than one RFC open at a time without mixing of issues/PRs occurring (since GitHub only supports having a single fork of a repository).
Copy link
Member

Choose a reason for hiding this comment

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

Is this a big issue? Have there been instances in the past where the same person is authoring multiple RFCs simultaneously? And if so, would labels on both issues & PRs by RFC number not solve this problem satisfactorily?

Copy link
Member

Choose a reason for hiding this comment

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

Unclear. Yes, e.g. 45/46. Maybe.

To be fair, line comments get auto-obsoleted on updates but issues are not; so if a full rewrite test-wise (which 46 has seen) would need probably need labelling even within a single RFC.

Copy link
Member Author

@infinisil infinisil Jan 8, 2024

Choose a reason for hiding this comment

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

I'm kind of already using this approach and I already have 4 RFC repos :)

I think it would be much cleaner to separate all RFCs with a repository. There's various benefits, like being able to infer that an RFC is likely ready for FCP once issues and PRs are at 0. Or not having to worry about having the correct label. Or the workflow not changing when somebody opens another RFC (before you wouldn't have needed a filter, but now you do), etc.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that makes sense. Might not be worth exploring this alternative, in that case.

- (+) For time-distinct RFCs it can be worked around by [detaching the old fork](https://support.github.com/request/fork) and creating a new one
- (+) Simplifies the RFC process, since one can just create a PR to upstream it
- (-) However this may again lead to the original problem of having too long PR discussions
- (-) It's also confusing about whether a PR is needed

# Unresolved questions
[unresolved]: #unresolved-questions


# Future work
[future]: #future-work

- More RFC automation is possible in the future:
Copy link
Member

Choose a reason for hiding this comment

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

In the spirit of automation, I think there will be a strong need of a template repository for RFCs, especially if a label architecture would be enforced as I mentioned in this comment.

- Creating repositories for discussing issues created in the rfcs repository
- Assigning the shepherd team and requiring them to review PRs to the repository
- Announce FCP when all issues/PRs are closed
- Commit contents to rfcs repository once FCP passed without any new issues/PRs