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

IPIP 0001: Lightweight "RFC" Process for IPFS Specifications #289

Merged
merged 10 commits into from
Jul 8, 2022

Conversation

lidel
Copy link
Member

@lidel lidel commented Jun 14, 2022

Closes #286

This PR includes:

  • IPIP 0000: Improvement Proposal Template
  • IPIP 0001: Lightweight Improvement Proposal Process for IPFS Specifications

This PR does not include repo cleanup – that will be a separate PR (to avoid noise here).

Feedback will be extremely valuable:

  • is this too formal? too informal?
  • anything missing?
  • anything can be dropped?

@lidel lidel self-assigned this Jun 14, 2022
Copy link
Contributor

@mishmosh mishmosh left a comment

Choose a reason for hiding this comment

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

Nice! IMO you've nailed the goal of making a clear and legible process, but keeping it minimal.

2 minor process questions, and a few minor grammar suggestions.

RFC/0000-template.md Outdated Show resolved Hide resolved
RFC/0000-template.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
Copy link
Member

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

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

How will existing RFCs be updated:

  1. when making editorial changes?
  2. when fixing a bug in the RFC?
  3. when adding a (minor?) new feature to a protocol?

Presumably 1. could be made by editing the RFC via a PR. For the 2nd and 3rd point, it's not clear if those would require a new RFC (which can get complicated, leading to multiple RFCs stacked on top of each other).

In libp2p, our RFCs have versions, so that in order to understand any protocol, it is sufficient to read a single document.

@lidel
Copy link
Member Author

lidel commented Jun 15, 2022

@marten-seemann the purpose of RFC documents in the process proposed here, is to document motivation and the change applied to the spec, not to be the spec itself.

We do want each spec to be a single document that is always up-to-date. We don't want to end up with IETF RFC situation where RFC==specs and you need to know "which RFC is the latest". The difference in the process proposed here is that RFC lives next to the specs.

To illustrate:

  • In order to understand how (hypothetical) WebDAV Gateway works, one would read contents of specs in ipfs/specs/WEBDAV_GATEWAY.md.
  • RFC in ipfs/specs/RFC/000N-webdav-gateway.md would only include Motivation and explainer why certain design decisions were made at a certain point in time. Initial 000N-webdav-gateway.md would explain why we added WebDAV spec in the first place.
  • If we realize the spec has a bug, we will evaluate the impact: adding more detail, editorials, cosmetics can be fixed without RFC.
  • Things that could cause an interop issues require a PR with fix and RFC doc in ipfs/specs/RFC/000M-webdav-fix-for-foo.md explaining why we make the breaking spec change, compatibility/migration considerations etc.

I went with RFC to avoid naming bikeshed, but we could rename these documents from "RFC" to something else, if it causes too much confusion.

Would CR (Change Request) be more clear?

Copy link
Member

@dignifiedquire dignifiedquire left a comment

Choose a reason for hiding this comment

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

looks like a great step forward 👍

@ajnavarro
Copy link
Member

ajnavarro commented Jun 15, 2022

I like it a lot! 🥳 I would add also non-accepted RFCs with the reason for future references.

@marten-seemann
Copy link
Member

marten-seemann commented Jun 15, 2022

@marten-seemann the purpose of RFC documents in the process proposed here, is to document motivation and the change applied to the spec, not to be the spec itself.

We do want each spec to be a single document that is always up-to-date. We don't want to end up with IETF RFC situation where RFC==specs and you need to know "which RFC is the latest". The difference in the process proposed here is that RFC lives next to the specs.

That makes a lot of sense, thanks for the clarification. Looks like I got confused by IETF terminology here.

I went with RFC to avoid naming bikeshed, but we could rename these documents from "RFC" to something else, if it causes too much confusion.

I'd appreciate that. Of course you're totally right that a "request for comment" quite literally should request comments, and not be the spec, but I fear that many people came across IETF RFCs and I wouldn't be the only one confused here.

Would CR (Change Request) be more clear?

Sounds good to me. Or maybe call it IPIP (InterPlanetary Improvement Proposal)?

RFC/0000-template.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
@BigLep
Copy link
Contributor

BigLep commented Jun 16, 2022

@lidel : thanks for putting this forward!

The answers in #289 (comment) about the relationship of RFC md's to Spec md's. Should we maybe put it in RFC/0001-lightweight-rfc-process.md so it's clear for future readers?

RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
RFC/0001-lightweight-rfc-process.md Outdated Show resolved Hide resolved
@ajnavarro
Copy link
Member

Putting some ideas here from a previous conversation with @lidel (cc @BigLep ):

WDYT about merging also RFCs that were not accepted, and also RFCs that are OK but will not be implemented yet (giving the opportunity to other IPFS implementations to do it if they think they are useful)?

Maybe we don't think that a specific spec is useful for us right now, but it can be useful for someone. We can add it as valid but not being officially part of the protocol. This will open a door for other IPFS implementations to implement it if it is worth it for their use case. Of course, those specs must work on top of the actual, validated ones, and be backward compatibles.

If a spec is well written but is invalid just because there is something better to do the same thing, or because it will break other specs (those are just 2 examples), we can merge them as rejected specs. With that, we can avoid common requests in the future and we can easily point to the rejected specs with the whys. If instead of doing that we keep pull requests open, we are coupling specs to Github. We will lose information if we want to generate some webpage with the specs, as an example.

Examples of this:

@ajnavarro
Copy link
Member

ajnavarro commented Jun 16, 2022

Or maybe call it IPIP (InterPlanetary Improvement Proposal)?

@marten-seemann I love that name.

@marten-seemann

This comment was marked as resolved.

@lidel lidel mentioned this pull request Jun 20, 2022
2 tasks
lidel added a commit that referenced this pull request Jun 20, 2022
@lidel
Copy link
Member Author

lidel commented Jun 20, 2022

Thanks for the feedback so far! Pushed some changes:

  • Renamed to IPIP (InterPlanetary Improvement Proposal)
    • This removed any assumptions people may have (RFC means different things to different people, IPIP will be unique)
  • Clarified the purpose and the life cycle based on git pull-requests
  • Created @ipfs/specs-stewards group responsible for reviewing IPIP/

@lidel lidel changed the title RFC 0001: Lightweight RFC Process for IPFS Specifications IPIP 0001: Lightweight "RFC" Process for IPFS Specifications Jun 20, 2022
@lidel
Copy link
Member Author

lidel commented Jun 20, 2022

Open question:

  • How do we handle IPIPs that are either rejected or deferred?

My take: IPIP pull request on Github in good enough (7e143b4). No need to introduce anything new.

Rationale:

  • IPIP is a diff against existing specs.
  • We can't merge "rejected" or "deferred" ones, as they may be in conflict with "approved" ones. Duplicating specs with applied delta for every rejected spec is not only work, but also adds noise, which we want to avoid.
  • Keeping "rejected" IPIPs as closed PRs, and "deferred" ones as open PR drafts in ipfs/specs repo is a compromise: it provides us with a linkable audit log of "things that did not make the cut" without introducing noise to the specs repo itself.

Maybe we don't think that a specific spec is useful for us right now, but it can be useful for someone. We can add it as valid but not being officially part of the protocol.

Realistically, given that we are still discussing what MVP IPFS is, none of the specs will be "required part of the protocol". Implementers will pick and choose parts depending on their needs and the level of interoperability with IPFS ecosystem their implementation requires.

I imagine we will be keeping IPIP PR open until the author continues working on it, addresssing feedback from community and stewards. When ready, we make a decision to merge or close without merging + provide rationale why.

My assumption is that IPIP will either be useful enough to be merged (ready to be used by implementers), or not. Hard to imagine the grey area where we merge something we are not happy with, but let me know if you have a more fleshed out example we could discuss.

@ajnavarro
Copy link
Member

ajnavarro commented Jun 20, 2022

My assumption is that IPIP will either be useful enough to be merged (ready to be used by implementers), or not. Hard to imagine the grey area where we merge something we are not happy with, but let me know if you have a more fleshed out example we could discuss.

I think I didn't explain myself well. My point was not to merge something we are not happy with, but something that is useful to someone, but is not part of the main IPFS implementation, and it is not needed to make a client work properly with the network.

Real example: BEP44: https://www.bittorrent.org/beps/bep_0044.html.

It is a draft, not accepted BEP, but it is really useful and there are several clients implementing it, and it works. When you try to do a PUT query, you will see that most of peers won't accept it, but you eventually will be able to save random data on the torrent DHT network because some of the clients are supporting that BEP, even being a draft. It is so useful that you have other BEPs based on that one, like https://www.bittorrent.org/beps/bep_0046.html.

@lidel lidel mentioned this pull request Jul 1, 2022
12 tasks
@lidel lidel force-pushed the feat/add-light-rfc-process branch from 7e143b4 to f65babb Compare July 8, 2022 18:41
@lidel lidel force-pushed the feat/add-light-rfc-process branch from 23f7cc2 to b29f325 Compare July 8, 2022 18:54
lidel added a commit that referenced this pull request Jul 8, 2022
@lidel lidel force-pushed the feat/add-light-rfc-process branch from dd0cbba to b0d6b2c Compare July 8, 2022 19:32
@lidel
Copy link
Member Author

lidel commented Jul 8, 2022

Thank you all for the feedback 🙏
I agree, it is not ideal, but better to have something to improve upon in follow-up PRs and IPIPs.

ps. @ajnavarro let's test drive the process for a few weeks first, and use PR drafts for "useful things that we are unsure about". When we have a good IPFS example of something similar to BEP44, then we will make a decision how to handle it, and will document the policy in README. (I am open to keeping them around, we just need a real example first).

I am merging this as there are no unresolved threads,
and we want this to be in the main branch in time for https://ipfs-thing.events next week.

@lidel lidel merged commit 8e45daf into main Jul 8, 2022
@lidel lidel deleted the feat/add-light-rfc-process branch July 8, 2022 19:37
@ajnavarro
Copy link
Member

@lidel sounds good to me.

@BigLep BigLep mentioned this pull request Jul 26, 2022
68 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Ratified
Archived in project
Development

Successfully merging this pull request may close these issues.

Lightweight RFC Process
9 participants