-
Notifications
You must be signed in to change notification settings - Fork 318
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
CIP-0001 | Rework to reflect reality #331
Conversation
I can't give this a full read & review before the upcoming retrospective meeting but at first glance it looks excellent & I'll catch up with it in the next day. Recently I realised how badly we needed this, especially with more community postings (for example) suggesting an inappropriately loose definition of "CIP" has been going around. It will be great to refer people to something more rigorous & updated 😎 |
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 a great step forward, and it really is like a complete rewrite... I'd been planning on reading the diff to focus on the changes, but the entire document is "diff" 🤓 so instead I read the old & new documents side by side.
I'm glad the Motivation section defines exactly how the CIP process relates to the highly marketable & overused term "governance". As long as I'm not misunderstanding this myself, this establishes that the CIP process is there to inform the governance process, not to achieve it.
I will try to remember the type "Standards Track" has been replaced by "Core" (a more intuitive term I think).
I admit that after >1 year I never really understood the CIP_Flow.png
diagram... and have seen many cases where it seemed we were jumping around independently of it, both in meetings and on GitHub... so I'm happy to see it finally gone.
I can see Repository organization includes the resolution we made, at the recent review meeting, to habitually double-check the CIP status fields ASAP after each PR is posted.
The Code of Conduct is adapted sensibly enough. I suppose its "Enforcement" section is necessary for liability reasons but it does park the "ownership" of the CIP process fairly well with the Cardano Foundation... this is something to consider whenever claims of impartiality or vested interest are discussed.
suggestions
The new PULL_REQUEST_TEMPLATE
is set to preemptively fix some trivial submissions and help start discussions on the Cardano Forum more productively, but:
- Question: is this in
.github
so it can be included automatically by the GitHub UI when a CIP pull request is created? - Suggestion: if so, can it also be linked from
README.md
text, and made visible in this CIP directory somehow (I don't see it in the directory listing on your branch)... so people can work with this material offline and/or on sites outside of GitHub (e.g. Discourse, e.g. the Cardano Forum)? Perhaps creating a symlink to make this available at the same location of the removed filecip-template.md
?
Under Repository organization we resolved at the last review meeting to add this item to the workflow, probably early on (at the time a CIP is first reviewed after PR is posted):
- If not already there, add a link in the original PR comment pointing to the CIP document in the author's working branch (for ease of discussion & community participation).
The list of current reviewers, with only 4 members, looks great from left-to-right: but won't be so when we have 10 editors, for which top-to-bottom would be better. I wish GitHub would generate responsive table markup... and perhaps it will by the time the editing committee grows to that size 🤣
People (including me, some time ago) wonder about cips.cardano.org
- how it's different from GitHub, how it's generated. A lot of this will be reverse engineered by experienced users going back & forth between the two environments... but for nontechnical people, writers, and managers it would also be helpful to have a short but explicit statement: perhaps one general enough that it will still be true even after the pending overhaul of this site.
- It could well be that this statement belongs in the top level
README.md
rather than in CIP-0001 itself. - I see the top level
README.md
is also being edited in this branch, and has removed the reference tocips.cardano.org
along with the whole text of the "Sequence Diagram" section, so perhaps it could be added back in under a sub-heading.
CIP-0001/README.md
Outdated
|
||
Pages must be named after the full CIP number (eg, "CIP 0001") and placed in the "Comments" namespace. | ||
For example, the link for CIP 1 will be <https://github.com/cardano-foundation/CIPs/wiki/Comments:CIP-0001>. | ||
CIP editors meet regularly (on a 2-week cadence) in [a public Discord server](https://discord.gg/qd6jE9Xj) to go through newly proposed ideas in a "triage" phase. As a result of a triage, editors acknowledge new CIPs, and briefly review their preamble. It is recommended not yet to assign a number and pick `?` when first opening a proposal. Instead, editors will allocate a temporary number as part of the triage phase. The triage also allows new CIPs to get visibility for potential reviews. |
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.
CIP editors meet regularly (on a 2-week cadence) in [a public Discord server](https://discord.gg/qd6jE9Xj) to go through newly proposed ideas in a "triage" phase. As a result of a triage, editors acknowledge new CIPs, and briefly review their preamble. It is recommended not yet to assign a number and pick `?` when first opening a proposal. Instead, editors will allocate a temporary number as part of the triage phase. The triage also allows new CIPs to get visibility for potential reviews. | |
CIP editors meet regularly (on a 2-week cadence) in [a public Discord server](https://discord.gg/gDabJZmd) to go through newly proposed ideas in a "triage" phase. As a result of a triage, editors acknowledge new CIPs, and briefly review their preamble. It is recommended not yet to assign a number and pick `?` when first opening a proposal. Instead, editors will allocate a temporary number as part of the triage phase. The triage also allows new CIPs to get visibility for potential reviews. |
Link has apparently been changed (old one is saying "invalid" at this time).
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.
When generating discord link, one has to be careful because they have a TTL of 7 days by default. There's a box to tick to make them last forever. The new one is already expired 🙃 ... I'll replace it with an infinite one.
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.
thanks; I'd seen that happen in other groups & was wondering why...
|
||
CIPs should be written in [Markdown](https://guides.github.com/features/mastering-markdown/) format. Each CIP should have the following parts: | ||
The CIP process does not _by itself_ offer any form of governance. Yet, it is a crucial, community-driven component of the governance decision pipeline as it helps to collect thoughts and proposals in an organized fashion. |
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.
I'd like to see this paragraph extended. This is one of the most common confusions I see. I'd like it to be really explicit that:
- Acceptance by the CIP editors does not indicate any judgement on the technical merit of a proposal.
- For this reason, a CIP being merged into the repository as Draft or Proposed means nothing other than that it is procedurally sound.
- The CIP process provides no guarantee or even process for getting a change accepted (into Cardano or otherwise as appropriate for the CIP). It is up to the author to find alternative channels to make this happen.
I'd also like to see a disclaimer like this on the CIP website. Maybe even a banner on Draft CIPs. At the moment stuff looks very official as soon as it's merged and it goes on the website.
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.
Acceptance by the CIP editors does not indicate any judgement on the technical merit of a proposal.
For this reason, a CIP being merged into the repository as Draft or Proposed means nothing other than that it is procedurally sound.
Actually, a CIP merged into the repository should also be technically sound; but because the editors can't technically review every CIP this is mostly a task that fall to the community. Editors can mediate the conversation and should ultimately have the last word based on what the various commenters said; but this is part of the goal.
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.
Perhaps this is also an issue for CIP-001: it's not clear to me what the criteria are for a CIP being accepted (also "accepted" isn't defined, I have been taking it to mean "merged into the repository").
I think the risk with saying that a CIP must be "technically sound" to be accepted is... by whose lights? Plenty of CIPs have been merged that, while reasonably coherent, were not IMO sound. Perhaps if the CIP editors were really forceful about ensuring that CIPs get expert reviews and didn't get merged until they were acceptable, that would help. But as it is I think many merged CIPs acquire a veneer of technical soundness which isn't necessarily justified.
So I guess my thought was to describe the CIP process as explicitly conveying less technical judgement, to allow well-written proposals to be merged even if they haven't been assessed. There's not really a clear way to justifiably say that a proposal is sound, so maybe we just shouldn't try 🤷
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.
(points noted regarding the definition of the statuses, I will refine that).
Regarding technical soundness of CIPs, this is something we strive for and part of the reason why some CIPs take so long to move forward. We need reviews from technical folks especially on areas where editors sole knowledge do not suffice.
Note also that some CIPs do not even have an explicit ownership when it comes to who should really approve (that's the case of pretty much any standard sitting atop of Cardano). Some others like Plutus related changes can be delegated to the Plutus maintainers for ultimate approval.
In some cases, designated experts aren't available to share their feedback (e.g. CIP-0050 or CIP-0007). And, while as editors we do our best to get their attention, we can't force people to review. Thus, putting some proposal in a weird state where they are 'sound to the best of our knowledge'.
There's also the case of few CIPs like Cip-0025 which were written after the fact, to capture an ongoing practice. It is hard to argue with those ones as they're usually simply documenting an existing process. Perhaps the mistake was to simply accept them because of that, instead of pushing back and commenting on their shortcomings. Yet, I think it's still preferable to keep such standards in one place, even if not ideal, because this is also where we can iterate on them and come up with new approaches to replace them.
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.
Thus, putting some proposal in a weird state where they are 'sound to the best of our knowledge'.
I think this is the crux of it. I'm suggesting that, since checking for technical soundness is not really something the CIP process can assure in general, it should explicitly disavow any such stamp of soundness. Of course getting technical review and improving the process is highly desirable, but for any random CIP there cannot be a guarantee that it meets any particular technical bar. And that's okay!
CIP-0001/README.md
Outdated
Status | Description | ||
--- | --- | ||
[Draft] | An implicit status given to newly proposed CIPs that haven't yet been validated or reviewed. | ||
Proposed | Assigned to CIP that has been reviewed, accepted and now working towards acceptance. 'Proposed' generally means that implementation is ongoing or that adoption from various actors is missing before moving to 'Active'. |
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.
What about CIPs that are high quality and get merged but have nobody working towards them? Or is the idea that a CIP should not be accepted unless there is a clear path to active?
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.
The latter. But, it isn't impossible to have a path to active but not being actively worked on. The path to active should simply set the "next steps" for a CIP. Doesn't mean they need to happen immediately.
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.
Then maybe you should say "'Proposed' generally means that the CIP has a clear 'Path to Active' that is being actively pursued by some entity"?
CIP-0001/README.md
Outdated
[Draft] | An implicit status given to newly proposed CIPs that haven't yet been validated or reviewed. | ||
Proposed | Assigned to CIP that has been reviewed, accepted and now working towards acceptance. 'Proposed' generally means that implementation is ongoing or that adoption from various actors is missing before moving to 'Active'. | ||
Active | The proposal has completed all steps needed for its activation. Said differently, it means observable metrics showing its adoption in the ecosystem. | ||
Obsolete | The CIP was either retired, made obsolete by a newer CIP or abandoned. |
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.
What about CIPs where e.g. the implementation gets rejected? "Obsolete" doesn't feel like the right word to to cover all of "superseded", "retracted", "rejected", etc.
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.
There used to be a status "rejected" which I dropped here in favor of only Obsolete. That's perhaps not a wise decision.
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.
Maybe just "inactive"?
This is way better! |
Perhaps this repository needs a CONTRIBUTING: obviously for contributing CIPs themselves, it should link to CIP-001, but there are clearly a few other things to say, like how the website works and so on. |
@michaelpj @KtorZ besides #331 (comment) (a deep link that may not last if that conversation is resolved) we now have #336 coming in support of having a |
Yes, Michael did it on purpose to force the re-introduction of a "rejected" status. Smart move. |
Honestly I just forgot you were going to remove it 😆 |
Had a little chat about the "rejected" status wrt #336. I think that "rejected" is not quite right, since it's not the case that the CIP process has rejected this proposal. What's more accurate is to say that it is no longer supported by its author. So something like "withdrawn" is maybe more appropriate. However, if you want to have a single "dead" status my vote still goes to "inactive", perhaps with the suggestion that an inactive proposal should have a short note explaining why it is inactive? "Inactive: no activity", "Inactive: withdrawn by proposer", "Inactive: superseded by X"? |
Looking back at how we historically handled things: I don't like inactive, nor do I like rejected (for the reason you mention). There has been only a couple of cases where we had a desire from an author to merge an "anti-CIP"; or said differently, to merge a purposely bad idea to keep a track history of discussions that happened. CIPs that are rejected are typically closed without making it to the repository. Once again, I am not saying that this is the process we want in the long-run, but it is the process we follow de facto. Since the action of proposing a purposely wrong CIP reflects an actual author's will, I think the status should also. Something that expresses "contemplated but rejected". Maybe: discarded or dropped ? |
I think there could be other workflows. Consider e.g. a pretty good CIP that gets merged, has lots of discussion, but then eventually it turns out that for some obscure incentives reason it can't be implemented in current Cardano. We probably don't want to delete it, but it would be nice to mark it in some way that makes it clear that it's not a live proposal any more. |
I would suggest "Invalid" (or "Declined") if not for the implication that people should be prevented from doing things differently than we've documented in CIPs. "Disavowed" may be closer but sounds overly dramatic, like from a spy movie... as does the ambiguous "Sanctioned" which also has the same problem as "Invalid". "Challenged" could incorporate all the possibilities above why a CIP "didn't make it"... but doesn't answer the next question in the mind of the reader: Why was it challenged? So, since none of these words will ever really tell the whole story, no matter how specific one or several choices would be, I also think the only option remaining is to add a qualifying phrase to |
As far as I know, I wasn't given a chance to comment about the reason for my last commit... which was to keep this fresh after today's omnibus update to the top level |
Opted for the following statuses rework:
|
A few thoughts on that:
|
@michaelpj keep in mind that this first-round of rework only aims to reflect the existing process (hence why I kept |
Ah I see, then ignore me! |
I was also going to say that not yet |
I've included most feedback (I hope) in this latest version. Note that I have left the discussion / rework of the statuses for the next iteration -- the one that'll define the new process. In this one, I have left the previous statuses as they were defined (and simply removed 'On Hold' which we literally never used). Tried to clarified the wording on the statuses thanks to @michaelpj's feedback. I hope we can get this one on review next week and merge it A.S.A.P. so we can have a proper discussion on what the CIP process should become next. |
This is the first step in a broader work aiming at pushing the CIP in the forefront of _every_ Cardano changes. Before addressing current shortcomings of the process, it is necessary to capture what the process _actually is_ at present. This rework will serve as a base for a retrospective discussion amongst the editors and, shall be proposed to the community for approval and review.
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.
Fine for this round, but still:
1 - By the time the final version comes out, prospective CIP authors should be able to see the CIP-TEMPLATE.md
file outside the PR submission process... since the effort for less experienced authors to submit a CIP will begin long before they are ready to submit a PR.
- Some people will be able to find it from the line "A reference template is available in
.github/CIP-TEMPLATE.md
" but I missed this myself in the last 2 readings... maybe because of the current right-alignment (why is it aligned like that?) - Nothing will be more obvious or useful to people than finding a link to or copy of this file somewhere they can see it: in the directory listing for CIP-0001 on GitHub.
2 - If we ask CIP submitters to include a link in their PR's OP to the proposal text (README.md
) in their branch, it will distribute the overhead we currently have for doing this... and more importantly reduce the broken links we'll have as authors rename their files & directories without updating any link we added ourselves (without their consent or perhaps even their knowledge) in the triage process.
@rphair I think that a PR template that (a) reminds of CIP-0001 should address your concerns right? I did remove the previously PR template on the basis that indeed, we do not want people to generally put the CIP's content in their PR body and thus, having the CIP template defined as a PR template felt off. Or are you suggesting that we should encourage people to do exactly as such? (reading your last comment, I am not sure anymore). I can also add a link to the CIP-TEMPLATE.md somewhere on the root README, to make it more apparent. |
(scratch last comment, I think we are reading each other wrong; going to re-post) |
thanks @KtorZ, the content & proposed usage of the template are fine... I'm only suggesting that this PR template itself should be something that people can read and find easily, regardless of how it's used or not used in the PR submission process. i.e. showing it somehow in this directory in which the contents of the I do agree, and have never disputed, that stuffing the template into a PR form would encourage the CIP going into the OP which has always been messy. 😎 |
p.s., hopefully explaining myself better this time 😖 ... I'm happy with & approving this draft either way, yet also thinking from the point of view of those who only want to learn (and possibly write) about the CIP process without actually writing a CIP. It will help these visitors to CIP-0001 to see the structural template in the directory listing, and not have to look for & possibly miss it in a document reference that currently looks like a footnote. 🤔 |
Merging this one now, to make room for the actual rework and CIP-9999. |
This is the first step in a broader work aiming at pushing the CIP in the forefront of every Cardano changes. Before addressing current shortcomings of the process, it is necessary to capture what the process actually is at present.
This rework will serve as a base for a retrospective discussion amongst the editors and, shall be proposed to the
community for approval and review.
see rendered Markdown