Doc: initial RIOT developer memo + directory structure#6191
Doc: initial RIOT developer memo + directory structure#6191emmanuelsearch merged 1 commit intoRIOT-OS:masterfrom
Conversation
|
Some more description or a pointer to a discussion about the purpose of this would be helpful. |
|
Well, how about we discuss this right here, as described in the current version of RDM0, available in this PR? See section 1. in doc/memos/rdm0.md |
|
I think usually if you propose a new feature in a PR it's helpful to state the purpose of this feature in the PR description. Apparently, here you are proposing a RFC memo format that can be used to document certain procedures, interfaces, BCPs etc for RIOT, right? |
|
Right. |
|
This is somehow similar to TEPs, right? I remember that we discussed if we want to have something like this for RIOT some time ago and the conclusion was that we weren't sure if this may introduce too much overhead or not. Are there any recommendations what exactly should be described in these memos? |
doc/memos/rdm0.md
Outdated
|
|
||
| # 1. Introduction | ||
|
|
||
| In order to facilitate RIOT maintenance in the long term and at large scale, additional documentation complementing code and usual code documentation is needed. For instance, such memos are expected to describe and give explanations about architectural design decisions, code structure etc. Other memos might also describe other aspects of RIOT activity, including, but not limited to, RIOT community processes, position with respect to some related external technical debates etc. |
There was a problem hiding this comment.
I would prefer if these files adhere the 80 character per line limit.
doc/memos/rdm0.md
Outdated
| Each memo is proposed as a pull-request (PR) and discussed the same way RIOT code PRs are processed on GitHub. | ||
|
|
||
|
|
||
| Once PRed, a memo must have received an ACK by at least 3 maintainers other than the author(s) of the memo, before its publication. |
|
What is IMO missing is some statement about when such a memo/RFC is required. Do I have to write a memo first before I want to introduce a new interface or component? Also, should one cite one or more memos in a PR? |
|
Concerning overhead: such memos are too much overhead compared to what? |
To not having them. 😉 I wonder if this discussion should not rather happen on the mailing list. These things tend to get overlooked by many people in the issue tracker. |
|
I'm not sure if we should "standardize" when a memo is needed. |
|
I just wonder if people will really write these memos voluntarily, but I guess future will tell us. |
|
concerning moving this discussion to the mailing list: it would defeat the process I'm describing in this PR (which relies on typical PR discussion on GitHub) so I guess you'll understand I'm not voting for that ;) |
|
No one does anything without a purpose, indeed ;) |
Not sure this will work, but we will find out. However, I think we may use Github-only for proposing these memos (if we agree to go with this approach) in the future, but for having the initial discussion about this approach itself, I would think that the mailing list would be the best place.
Well. there's a maintainer mailing list, but actually, I tend to disagree here: the target audience here is the whole RIOT community. |
The target audience of the memos is the whole RIOT community, agreed. |
|
Do we need this kind of formalism? I imagine a change that I want to introduce which is serious enough that it would apply for an RDM. Usually, after a long period of subconsious planning/architecting, I start writing a prototype implementation to see if the idea is viable. If it is, I clean it up, then create a PR (with RFC tag). People take a look, comment, bash, critizise, gratulate, help, ... and possibly after a while, the code is clean and documented and the general idea is accepted. The PR ripes, and at some point, we merge. Now at what point would I write an RDM? |
|
How about "retrospect" documentation, such as the device driver guide and the series of BCP and design rationale docs like that on the HAL that @haukepetersen is in the process of writing? |
Well, definitely nice to have. But unfortunatly not for free. I know I'd hate writing these documents. Let's not forget that they say TEP's had a big part in TinyOS' slow and painful death. ;) |
|
(just realized that it would be better if the memos had titles ;) |
|
@kaspar030 well no one forces you to write a memo you don't want to write ;) |
But that was because TEPs were mandatory for implementing a new feature or releasing a new version, right? From my perspective it's gonna be interesting to see if we find a good trade-off between not making this mandatory and still motivating developers to actually write these documents. |
I'd still be up for this, but agreed that we should get it in and start using it. I'll go with the momentum and... ACK. |
|
@danpetry OK (you need to ACK through the dedicated reviewer interface in GitHub ;). |
|
@smlng you OK to run with this for now? |
doc/memos/rdm0.md
Outdated
| # 3. Memo Publishing and Maintenance Process | ||
|
|
||
| Each new memo MUST be proposed as a pull-request (PR), | ||
| with temporary number RDM 99999, |
There was a problem hiding this comment.
Why a temporary number? It will make it hard to refer to the memo before it is accepted, especially if it is referred to from other pending memos as well.
Maybe we could start by numbering it 99999 and immediately get a maintainer to assign it a proper number while still in review stage.
There was a problem hiding this comment.
@jcarrano in the end we want a nice series of published RDMs.
Imagine someone starts RDM4 and it never gets published.
In the mean time RDM5 is published... would look bad.
So obviously temporary numbers are the way to go.
And aside of their number, we can refer to RDMs under review by their title, by their PR number, by an URL to the PR... so no issues here I think.
There was a problem hiding this comment.
I would vote to only assign the number at the very end of the review. Otherwise we might have big wholes in our RDM list if an RDM get's rejected after all. Maybe something similar to Internet drafts would be better here: RDM-draft-<author>-<topic>. This way it is at least somewhat referable.
There was a problem hiding this comment.
@miri64 so for this RDM you mean "RDM: draft-baccelli-rdm-format" in the meta-data instead of "RDM 99999" (which it is not using btw ;)?
There was a problem hiding this comment.
Or we keep rejected RDM for "memo" reasons. As experienced quite often, it is nice to be able to point to a reference when rejecting changes.
There was a problem hiding this comment.
In the mean time RDM5 is published... would look bad.
Do we care about the looks?
Does that matter? Internet RFC's aren't published in numerical order, either.
There was a problem hiding this comment.
rejected RDM are kept automatically as closed PRs. Is that enough?
There was a problem hiding this comment.
@kaspar030 actually RFCs are published in a way such that there are mostly no holes in the numbers of published RFCs. And: RFC numbers are assigned by the RFC editor, just before publication (not at the time of fiddling with drafts ;)
There was a problem hiding this comment.
@miri64 so for this RDM you mean "RDM: draft-baccelli-rdm-format" in the meta-data instead of "RDM 99999" […]?
Yes, e.g, but also the file name being rdm-draft-baccelli-rdm-format.md
(which it is not using btw ;)?
I think RDM0 is a special case here ;-P
There was a problem hiding this comment.
@miri64 added specification of temporary name à la Internet Draft.
(immediately squashed, and rebased)
doc/memos/rdm0.md
Outdated
|
|
||
| Bigger changes should result in a new memo with another | ||
| RDM number, deprecating the old memo. In this case, the deprecated memo must | ||
| be modified to append a line in its header indicating "DEPRECATED BY RDM X" |
There was a problem hiding this comment.
Or we can have a "Status:" field similar to Python's PEPs.
There was a problem hiding this comment.
I think Deprecated and non-Deprecated is enough. Python has a number of Statuses and I think this would be overkill - it'd add work and process, and thereby make it more difficult for us to support them
There was a problem hiding this comment.
Maybe also add a link then: "[DEPRECATED BY RDM X](./rdmXXXX.md)"?
There was a problem hiding this comment.
I was also thinking of Rust's RFC or Scheme SRFI. These go through a draft stage and then can become final or withdrawn.
My point is: how to have a solid proposal that is being worked on not stay in a PR? (without forcing it to be final)
doc/memos/rdm0.md
Outdated
| and discussed the same way RIOT code PRs are processed on GitHub. | ||
|
|
||
| Initially, a proposed RDM is identified by a temporary name | ||
| similar to IETF Internet Draft names (for |
There was a problem hiding this comment.
A short generic definition (beyond the example) would be great here. Not everyone nows how Internet Drafts work.
There was a problem hiding this comment.
+1, could even remove the reference to IETF and just describe it, in case we want to deviate. IMHO we could (should) take pointers from the IETF but not stick to them "because that's what the IETF does" - directly referring to the IETF suggests the latter to some extent
miri64
left a comment
There was a problem hiding this comment.
Since this PR is only missing one ACK now, here is my nit-pick review (including some last-minute format suggestions) ;-)
doc/memos/rdm0.md
Outdated
|
|
||
| - RDM: 0 | ||
| - Title: RIOT Developer Memo Format, Publishing and Maintenance Process | ||
| - Author: Emmanuel Baccelli |
| @@ -0,0 +1,153 @@ | |||
|
|
|||
There was a problem hiding this comment.
Something to consider: maybe use rdm0000.md as a filename. That would make sorting easier beyond 10 RDMs ;-).
There was a problem hiding this comment.
(I'm only talking filename here; I don't see any benefit in making that also mandatory for e.g. "RDM 0" when used in the text below).
doc/memos/rdm0.md
Outdated
|
|
||
| ## Status | ||
|
|
||
| This document is a product of the community of RIOT maintainers, and aims to |
There was a problem hiding this comment.
More trailing whitespaces (I won't mention more below).
doc/memos/rdm0.md
Outdated
| of RIOT, a memo is considered published. | ||
|
|
||
| Small changes/clarifications may be subsequently PRed to improve a memo, using | ||
| the same RDM number. In this case, a (monotically increasing) revision number |
doc/memos/rdm0.md
Outdated
|
|
||
| ## Contact | ||
|
|
||
| The author of this memo can be contacted by email at emmanuel.baccelli@inria.fr |
There was a problem hiding this comment.
Pretty sure the wording should be "via email", but English is not my native language ;-).
There was a problem hiding this comment.
I usually say via, but probably both are grammatically correct
There was a problem hiding this comment.
P.S. we could perhaps start using something like this to check documents and make them read more professional-like
There was a problem hiding this comment.
Maybe rather use something that is integrated in the GitHub check system. AFAIK, grammarly is not.
There was a problem hiding this comment.
yeah sure. I was more thinking that people could use it themselves to assist manual checking.
doc/memos/rdm0.md
Outdated
| targets, | ||
| 3. memos are expected to be of normative and durable nature, hence: keep a memo | ||
| as concise as possible without impairing clarity, and factor out content that | ||
| would be likely to be updated too often. |
There was a problem hiding this comment.
Emmanuel is grammatically correct but the sentence is a little convoluted. I'd say "is likely to quickly become out-of-date"
doc/memos/rdm0.md
Outdated
| 3. the list of revisions so far and corresponding changelog, | ||
| 4. contact information to reach the author(s). | ||
|
|
||
|
|
There was a problem hiding this comment.
Are these two new-lines intentionally (sorry, sometimes there are two newlines between sections, sometimes there aren't, sometimes between paragraphs; the pattern isn't clear to me)
doc/memos/rdm0.md
Outdated
|
|
||
| Bigger changes should result in a new memo with another | ||
| RDM number, deprecating the old memo. In this case, the deprecated memo must | ||
| be modified to append a line in its header indicating "DEPRECATED BY RDM X" |
There was a problem hiding this comment.
Maybe also add a link then: "[DEPRECATED BY RDM X](./rdmXXXX.md)"?
doc/memos/rdm0.md
Outdated
|
|
||
|  | ||
| <p align="justify"> | ||
| *Figure 1. The RIOT logo.* |
There was a problem hiding this comment.
Since you changed into HTML-context, the *s are not rendered properly.
smlng
left a comment
There was a problem hiding this comment.
I will not block this, but I'm still in favour of a separate RDM repository and would also encourage to re-use the format, tooling, and processes how the Python community handles their PEPs. See:
| - Title: RIOT Developer Memo Format, Publishing and Maintenance Process | ||
| - Author: Emmanuel Baccelli | ||
| - Date: October 2018 | ||
|
|
There was a problem hiding this comment.
referring (again to Python PEPs) I would add some more header fields such as type, e.g. Informational, API, ..., and status e.g. Draft, Active, Deprecated. See https://github.com/python/peps/blob/master/pep-0001.txt
There was a problem hiding this comment.
also content-type might be helpful
doc/memos/rdm0.md
Outdated
| memo. Bigger changes should result in a new memo deprecating the old memo. | ||
|
|
||
| A dedicated directory in RIOT codebase (RIOT/doc/memos) contains all the memos. | ||
| It is acceptable to add graphical content (in RIOT/doc/memos/graphics/) to |
There was a problem hiding this comment.
regarding directory structure, why not have a more generic files subfolder with subfolders per RDM (folder name = rdmXXXX) and common folders for shared files. I think there might be other things than graphics (e.g. code snippets, style sheets for rendering) only that can be stored there.
|
Another advantage for keeping it in the repository is that we can organize the subject of the RDM in a repository project together with the RDM PR. Having it in a separate repo would require an Orga project, just to include the RDM. |
this only holds as long as all RIOT related code is in one repository. But IIRC there are many ideas and lots of effort in the direction of allowing external code (drivers, boards, etc) to be more easily integrated with RIOT. |
That doesn't change the fact, that with putting it in a separate repo we always need an orga repo. This would include when there is a new effort to make a new featureX within the RIOT repo (while when the RDM is here and still under discussion while some initial implementation work is done it could all live in this repo). I expect that most of the RDM-triggered features are large enough efforts to justify a project, so this is something we should consider. |
|
I fear that handling RDM in the RIOT repository will stall their progression from draft to standard (or what ever we want to call it when they are finished), because we currently have 400++ open PRs and thus RDMs will just be some more on top of that number. Having a separate repo for this will make the (editorial) process of reviewing and shaping an RDM to its final state (IMHO) easier and more visible. Also I think, there are community members not so much involved in adding feature to RIOT (i.e. coding in C) that might help here as document shepherds (see IETF) or others that contribute tooling around RDMs. Also, do we really want to have Murdock compile and test RIOT when someone just wants to PR and RDM? I know we can add (yet another) exception in our CI toolchains to work around that, but then we might want to run certain (style, grammar, format) checks on RDMs, too, which might not be useful for all the other files in the RIOT repo. |
|
+1 for an external repo, and it seems to me like the consensus in this discussion is towards that. We just had a discussion in the weekly meeting where the consensus amongst those present was towards merging it soon in its current state, and then iterating as we get a better feel for how people are using them. Including where they live, additions to the format, and additional processes. IMO this is a good approach which will allow us to home-grow our own RDM process and doc format - which is best suited to us - rather than re-use someone else's. |
118c3ef to
56db0c6
Compare
|
Addressed all comments (except moving to separate repo ;) |
miri64
left a comment
There was a problem hiding this comment.
I had a quick scan-through and didn't any blaringly obvious things to comment on, so ACK from my side as well.
Regarding the separate repo discussion, I think that the result of our meeting is the most sensible approach.
I already created a new Area: label Area: RDM.
|
Travis found another trailing white space though ;-). |
|
damn. trailing white space hopefully fixed now. Let's see Travis. |
|
everything green. 3 ACKs as per procedure. No showstoppers. Let's get this out of the way from the CI ;) |
|
@kaspar030 @danpetry @miri64 @haukepetersen and @allmaintainers bring on your RDM proposals! |
Doc: initial RIOT developer memo + directory structure
This PR proposes an RFC memo format that can be used to document certain procedures, interfaces, BCPs etc for RIOT.