-
Notifications
You must be signed in to change notification settings - Fork 276
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
libp2p specification framework – lifecycle: maturity level and status #169
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great - I like that the bar to getting an Initial Working Draft
is low, and having time be an explicit factor in the review and lifecycle process seems like a win to me.
I have no real opinions about the header format :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on putting all this together 👏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall I like this approach and the stages. My primary concern is around clarity of an accepted PR.
a Pull Request that is reviewed by the libp2p community in no more than
X working days.
I think it would be helpful to clarify the parameters of success here, and who is responsible for the merge. Are there certain individuals that need to approve, or is any libp2p community member able to approve? I would expect something along the lines of having sign off from at least 2 members of a defined list of spec maintainers to transition a spec from 1 stage to another.
Clarity on how to progress between levels is needed, that's very true. Especially if we're committing to X days for review and so on. |
@jacobheun @vasco-santos @yusefnapora How do you feel about the following:
|
@raulk that approach seems good to allow a smooth iteration between levels 👍 |
Calling them levels makes me think you always want to move on to the next level, but that's not the case here. Maybe we should use a word that doesn't imply succession like that |
@whyrusleeping the spec refers to the progression as stages. We used the term levels in discussion, but that's inaccurate. |
Would the Interest Group members be specified in the header of the spec? Also, 5 seems excessive, especially for the Initial Working Draft. I could see this being more reasonable for the two recommendation stages. We have historically dropped the ball on moving things forward. I think having designated individuals will help, but I also think the barrier to entry should be lower for Working Drafts. Maybe start with 3 and recommendations require 5? Do all Interest Group members need to approve the PR before merge? I think majority rules might be good here. PRs that have open request for changes, might be a reasonable blocker, as long as they don't get stale (at which point they should be dismissed imo). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good work, but I think the deadlines are too aggressive.
@momack2 I'm trying to modularise the spec framework; this one addresses the semantics of spec maturity and status. Specs repository structure and document naming are definitely elements worth speccing separately; thanks for bringing those up! So far we have converged to placing top-level specs in the root and subsystem specs in directories. People seem to be comfortable with this organisation, and we might want to formalise it in a spec. |
I see positive feedback for splintering "maturity stage" into separate "maturity level" and "status" fields. Here's the approach I'm taking:
|
This is very nice stuff! Are abandoned candidate recommendations moved to |
I believe I've now addressed your comments about the impreciseness of the term "stages", and the sequential numbering. We now have a "maturity level" (progressive) and a "status" field. See this commit: 9b630b9. @whyrusleeping @vyzo @bigs @yusefnapora and everybody else: could you review these changes and post any remaining concerns? Merge bus will be departing today 🚌🚌🚌 |
I've now split this up into two fields: maturity level and status. Could you check if the new revision is clearer? |
This is much improved since my initial hasty approval 😄 @raulk how would you like to apply this framework to the existing PRs? I'd like to get #106 and #100 merged soon. Perhaps it's best to pull them both in as Candidate Recommendations. Then we can have a sort of "practice round" of promoting them to Recommendations so we can get a feel for that process, and hopefully improve them some more while doing so. |
We have been talking about this for a long time, so I understand your excitement in merging it in ;-)
Off the top of my head, go over all outstanding PRs and:
I think #106 (SecIO) can be a 3A (Recommendation, Active). For #100, we can merge a spec that reflects the status quo as a 3A, with a notice that a refinement is on the way as a separate 1A (open a new issue to address the serialisation discussion?). Or we can merge it as a Working Draft to begin with. |
Yes, the status and the table showing its possible transitions are helpful. looks good to me! 👍 |
1 similar comment
Yes, the status and the table showing its possible transitions are helpful. looks good to me! 👍 |
Thank you all for your feedback! @yusefnapora – would you like to take the lead on a small
|
Why is an important spec like this merged after just two days of discussion? |
@marten-seemann the consensus has been overwhelmingly positive so far. We really need to be nimbler; a lot of people are frustrated with our general heavyweightedness (me included). Also, we've had prior discussions in this direction, so nothing here should be controversial. Feel free to review the PR after the fact, or open a new PR to modify this work. If anybody else feels they weren't able to get their input in in time, please say so. We can revert this PR and subject it to another round of reviews if necessary! |
I added a commit to #100 with a first-pass at a document header. If anyone has opinions about that sort of thing, please check it out and let me know. Once we have something we like we can add a little spec for the header format. |
@yusefnapora – that LGTM! |
@raulk I would like to reiterate @marten-seemann point but also propose the solution of explicitly stating the timeframe in which a contributor has to make a comment/suggestion. Even if consensus is reached before the end of the timeframe, the PR shouldn't be merged until the timeframe is complete, in case someone hasn't had the chance to comment, i.e. like @marten-seemann on this PR. The timebox will cut the long tail off getting things through, whilst also allowing contributors to feel that they had the opportunity to contribute. Personally, I would have like to see this PR open for at least 4-5 days so that there is ample time for it to makes its way around the globe. This PR only had one and half round-trips between EU and Asia, and due to other priorities, I wasn't able to even see if my comments from the first day were responded before the PR was merged. (sidenote: I have had look now and its all good) I agree that things speeding up is a good thing but there needs to be a predictable timing to how things go so that people can schedule the time in their weeks to review and do a good job of things instead of rushing because they don't know when something will be merged, because consensus was formed on the other side of the planet, whilst they were sleeping. EDIT: I will open an issue about adding timeframes to reviews of specs. |
A minimal viable meta-specification of the maturity process for libp2p specifications.
Next step is to specify the header for libp2p specification documents. It will carry at least: stage, author, current revision.
Context: some implementors are understandably frustrated because our spec process is too heavyweight. I suspect we are suffering from the psychological effect of not modelling spec maturity levels, so everything is either a binary “perfect or nothing”. Introducing spec maturity levels will dislodge stuff by allowing us merge docs that aren’t perfect, but are on their path to becoming so, and that’s fine.