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

CIP-0001 | Rework to reflect reality #331

Merged
merged 1 commit into from
Oct 26, 2022
Merged

CIP-0001 | Rework to reflect reality #331

merged 1 commit into from
Oct 26, 2022

Conversation

KtorZ
Copy link
Member

@KtorZ KtorZ commented Sep 21, 2022

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

@KtorZ KtorZ changed the title Rework CIP-0001 to reflect reality CIP-0001 | Rework to reflect reality Sep 21, 2022
@KtorZ KtorZ added the Update Adds content or significantly reworks an existing proposal label Sep 21, 2022
@rphair
Copy link
Collaborator

rphair commented Sep 21, 2022

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 😎

Copy link
Collaborator

@rphair rphair left a 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 file cip-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 to cips.cardano.org along with the whole text of the "Sequence Diagram" section, so perhaps it could be added back in under a sub-heading.


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.
Copy link
Collaborator

@rphair rphair Sep 22, 2022

Choose a reason for hiding this comment

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

Suggested change
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).

Copy link
Member Author

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.

Copy link
Collaborator

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...

@rphair rphair requested a review from dcoutts September 22, 2022 15:22

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.
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

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 🤷

Copy link
Member Author

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.

Copy link
Contributor

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 Show resolved Hide resolved
CIP-0001/README.md Outdated Show resolved Hide resolved
CIP-0001/README.md Show resolved Hide resolved
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'.
Copy link
Contributor

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?

Copy link
Member Author

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.

Copy link
Contributor

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"?

[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.
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just "inactive"?

@michaelpj
Copy link
Contributor

This is way better!

@michaelpj
Copy link
Contributor

People (including me, some time ago) wonder about cips.cardano.org - how it's different from GitHub, how it's generated.

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.

@rphair
Copy link
Collaborator

rphair commented Sep 26, 2022

@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 Rejected status. I also don't think it makes sense to mark such proposals as "deprecated" (since they were never in favour) or "obsolete" (since they were never in use).

@KtorZ
Copy link
Member Author

KtorZ commented Sep 27, 2022

Yes, Michael did it on purpose to force the re-introduction of a "rejected" status. Smart move.

@michaelpj
Copy link
Contributor

Honestly I just forgot you were going to remove it 😆

@michaelpj
Copy link
Contributor

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"?

@KtorZ
Copy link
Member Author

KtorZ commented Sep 27, 2022

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 ?

@michaelpj
Copy link
Contributor

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.

@rphair
Copy link
Collaborator

rphair commented Sep 27, 2022

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 Inactive as suggested in #331 (comment). It shouldn't be that hard to establish (starting with the 2 or 3 cases we've seen so far) a canonical set of choices to add after Inactive:.

@rphair
Copy link
Collaborator

rphair commented Sep 27, 2022

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 README.md of #338 & avoid further merge conflicts getting too hairy.

@KtorZ
Copy link
Member Author

KtorZ commented Oct 6, 2022

Opted for the following statuses rework:

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'.
Active The proposal has completed all steps needed for its activation. Said differently, it means observable metrics showing its adoption in the ecosystem.
Inactive The CIP is no longer, or has never been, active. When marked 'Inactive', the status should also indicate a reason (e.g. obsolete, superseded, abandoned...). For example: Inactive: superseded by CIP-XXXX

@michaelpj
Copy link
Contributor

A few thoughts on that:

  • Do we really need "Draft"? If the review process is implicitly the same as the PR process, isn't a draft CIP simply one that isn't merged? Would anyone ever put "Draft" in the CIP status field?
  • I still find the description of "Proposed" a bit difficult to understand. It's trying to distinguish itself from "Draft" as well as "Active". If it just meant "proposed but not yet active" then I think it would be easier to understand. "Any proposal which is not yet Active but which is actively being moved forwards towards Active"?
  • For Active, maybe it's helpful to distinguish the types of CIP? "For a Core CIP, the changes should have been implemented and released in the Cardano implmenetation, ..."

@KtorZ
Copy link
Member Author

KtorZ commented Oct 6, 2022

@michaelpj keep in mind that this first-round of rework only aims to reflect the existing process (hence why I kept Draft, although I do agree with you, this status is completely redundant as it simply describes anything in a PR). There's another PR coming soon with the actual CIP-0001 rework (for which I hope to capture most of your suggestions).

@michaelpj
Copy link
Contributor

Ah I see, then ignore me!

@rphair
Copy link
Collaborator

rphair commented Oct 6, 2022

I was also going to say that not yet Active proposals submitted as PR's would have to be triaged to include a Path to Active if they were submitted as Proposed... but @KtorZ I guess you are planning an new state that does not imply review but means more than the meaningless Draft 🤓

@KtorZ
Copy link
Member Author

KtorZ commented Oct 6, 2022

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.
Copy link
Collaborator

@rphair rphair left a 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.

@KtorZ
Copy link
Member Author

KtorZ commented Oct 6, 2022

@rphair I think that a PR template that

(a) reminds of CIP-0001
(b) reminds of CIP-TEMPLATE.md
(c) encourages people to also provide a rendered version of their CIP (using Github's existing capabilities)

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.

@rphair
Copy link
Collaborator

rphair commented Oct 6, 2022

(scratch last comment, I think we are reading each other wrong; going to re-post)

@rphair
Copy link
Collaborator

rphair commented Oct 6, 2022

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 .github directory are suppressed: https://github.com/cardano-foundation/CIPs/tree/cip-0001-rework/CIP-0001

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. 😎

@rphair
Copy link
Collaborator

rphair commented Oct 7, 2022

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. 🤔

@KtorZ
Copy link
Member Author

KtorZ commented Oct 26, 2022

Merging this one now, to make room for the actual rework and CIP-9999.

@KtorZ KtorZ merged commit d973534 into master Oct 26, 2022
@KtorZ KtorZ deleted the cip-0001-rework branch October 26, 2022 06:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Update Adds content or significantly reworks an existing proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants