Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

add: draft RFC template #488

Merged
merged 11 commits into from
Jul 25, 2022
Merged

add: draft RFC template #488

merged 11 commits into from
Jul 25, 2022

Conversation

kaiserd
Copy link
Contributor

@kaiserd kaiserd commented Feb 12, 2022

This draft PR demonstrates an example structure for RFCs of the WAKU standard category as described in this forum post.

@kaiserd kaiserd self-assigned this Feb 12, 2022
@jm-clius
Copy link
Contributor

I've commented on the wider issue of RFC structuring here: https://forum.vac.dev/t/rfc-categories-structure/125

title: 42/WAKU2-STANDARD-Template
name: Waku v2 Stadard Template
status: raw
category: WAKU standard
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

discussed during offsite, let's use categories a la IETF

Tags can be used as optional meta information but shouldn't be imposed as it creates complexity/overhead and there also doesn't seem to be a good precedence for it in specs world (IETF etc)

Copy link
Contributor Author

@kaiserd kaiserd Jul 16, 2022

Choose a reason for hiding this comment

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

We'd have to update the 1 RFC to introduce the category.

@kaiserd
Copy link
Contributor Author

kaiserd commented Jul 16, 2022

Updated as per our off-site discussion. This would make tagging, as introduced #368, optional.

An alternative/addition to tagging for clarifying the usage of various protocols are BCP (best common practice) RFCs.

@kaiserd kaiserd marked this pull request as ready for review July 16, 2022 14:56
@kaiserd kaiserd requested a review from D4nte July 16, 2022 14:56
@kaiserd
Copy link
Contributor Author

kaiserd commented Jul 16, 2022

We could add further information about writing RFCs in the README.md

@kaiserd kaiserd mentioned this pull request Jul 16, 2022
2 tasks
Copy link
Contributor

@jm-clius jm-clius left a comment

Choose a reason for hiding this comment

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

This LGTM! Thanks. Will go a long way to make our RFCs more standardised and readable.

@@ -0,0 +1,62 @@
---
slug: XX
title: XX/(WAKU2|LOGOS|CODEX|...)-TEMPLATE
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming RFCs that does not belong to a single project or applies to several (e.g. "RLN" without relay) would just not use a prefix at all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could make the prefix optional (i.e. not mentioning it in the template at all).
Or, we could list a set of "known" prefixes, but also allow further prefixes / prefix-less RFCs.
The [...] could be replaced by "*": any or no prefix. README could explain this.
wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good to me! Don't think we need too overexplain this - a "*" would fully indicate the meaning, I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

We might differentiate between project's specific RFCs from general RFC (that apply to several projects) with a notation like
XX/WAKU2/A-TEMPLATE <- specific to WAKU2
XX/ANOTHER-TEMPLATE <- multi project
This avoids the double meaning of the "-" which separates prefix and suffix, but also multi-words suffixes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that :).
What I did in the template was to be backwards compatible. But... I'd have to update the RFCs adding the category anyway. Ill put that into the restructure issue (yet to be opened, see #513)

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I'm worried this complicates things? The optional prefix seems like most minimal KISS.

Do we have an example of some spec project using double namespacing? E.g. //.

What do we hope to gain from this, vs more informal prefix?


# Wire Format Specification / Syntax

A standard track RFC must feature this section.
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps only when in draft or higher? Perhaps we may want to relax requirements for raw RFCs so we can standardise a bit quicker on semantics even if wire format is not yet fleshed out?

Copy link
Contributor Author

@kaiserd kaiserd Jul 19, 2022

Choose a reason for hiding this comment

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

Yes. The the MUSTs are for "stable". Raw and draft "should". Done in 4186eb8.

A standard track RFC must feature this section.
This section SHOULD not contain explanations of semantics and focus on concisely defining the wire format.
Implementations MUST adhere to these exact formats to interoperate with other implementations.
It is fine, if parts of the previous section that touch on the wire format are repeated.
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this. This section is in other words the TL;DR for implementers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. An implementor who already understand the basic functioning and background of a protocol can go to this section directly.

@kaiserd
Copy link
Contributor Author

kaiserd commented Jul 19, 2022

I added a meta section for explaining things about the process of writing RFCs (it also links COSS).
For now it explains tags as introduced in #368
Also using the tag path structure suggested by @easye in the forum discussion https://forum.vac.dev/t/rfc-categories-structure/125/3
Wdyt?

contributors:
---

# (Info, remove this section)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder, should this be a template or in itself a spec? :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I could remove this and put things into COSS exclusively.
However, Imo, like this the template is self-contained and I would add the info to COSS as well anyways.
(see my comment above). Could also change this in a future PR. Wdyt?

Copy link
Contributor

@oskarth oskarth left a comment

Choose a reason for hiding this comment

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

LGTM, and this will be helpful to spec writers!

Copy link
Contributor

@s1fr0 s1fr0 left a comment

Choose a reason for hiding this comment

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

LGTM too! Great standardization job! I left a comment regarding how we can possibly differentiate between project-specific and general RFCs.

@kaiserd
Copy link
Contributor Author

kaiserd commented Jul 22, 2022

Further opinions on @s1fr0 suggestion? I like that. My comment inline

* master:
  RFC16: add version call (#505)
  fix(noise): update RFC to implementation (#508)
  fixup: 37/WAKU2-NOISE fix images paths (#506)
  New RFC: 37/WAKU2-NOISE-SESSIONS (#504)
  36/WAKU2-BINDINGS-API (#501)
  docs(16/WAKU2-RPC): add ENR to waku info (#502)
  Adding 35/WAKU2-NOISE to menu (#500)
  add RFC33 to index (#499)
  feat: 32/RLN raw spec
  New RFC: 35/WAKU2-NOISE (#496)
  Update on the rln registration figure to match the current spec (#497)
  33/WAKU-DISCV5: Add first raw version (#487)
  Add pubsubTopic field to index (#492)
  Fix markdown links (#493)
  Categorize 22 & 31 (#490)
  Changed PB Timestamp index to 10 (#491)
  13/14/16/21: Change in timestamp format (#483)
  add: RFC31 copyright statement (#489)
  17/WAKU-RLN-RELAY: Revise spec for its draft version (#484)
@kaiserd
Copy link
Contributor Author

kaiserd commented Jul 25, 2022

(The merge commit done by via Github was not signed, so I had to rebase and force commit.)

@kaiserd kaiserd merged commit cc3f4ee into master Jul 25, 2022
@kaiserd kaiserd deleted the add/waku-rfc-template branch July 25, 2022 11:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants