-
Notifications
You must be signed in to change notification settings - Fork 321
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-0035 | Update for new CIP-1 #437
CIP-0035 | Update for new CIP-1 #437
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.
@michaelpj this is great and also helps for compliance with the new standardisation proceeding via #389. At first view the document structures all look parseable (headers at the right levels) but the new form also needs:
a Path to Active for each one, even ones that are currently Active
.
- This is undocumented (or implied) so far because https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#status-proposed only lists these criteria for
Proposed
proposals. - But we are adding this section for all merged CIPs as per Update older CIPs for revised CIP-0001 #389 (comment) and if adding a history of acceptance & implementation retroactively then nobody understands these better than you, so it makes the most sense for you to write it.
- @KtorZ if my understanding after several GitHub exchanges is that we must have Path to Active for every single proposal is incorrect, now would be a good time to correct it (and also to confirm it).
a section at the bottom with a link to the License quoted in the header (editors can edit these in later if missed here).
another nitpick which we might also handle in concierge style: each of the Motivation and Rationale sections now have slightly longer titles:
- Motivation: why is this CIP necessary?
- Rationale: how does this CIP achieve its goals?
... @KtorZ I feel sheepish asking people to add these as the literal section titles, especially considering what it does to the section anchors (instead of simple & immutable anchors like #motivation
), but these are mandatory section titles as per https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#structure.
CIP-0035/README.md
Outdated
Comments-Summary: No comments | ||
Comments-URI: |
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 CIP preamble needs the same makeover as CIP's 31 to 33 (some obsolete fields here). Note we will also be double checking this in #389.
CIP-0031/README.md
Outdated
Categories: | ||
- Plutus | ||
- Ledger |
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 duplicate category seems perfectly understandable to me but this syntax still awaits confirmation via issue #433 and probably also (@KtorZ) an update to https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#categories.
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's actually no Ledger
category.
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 mean, it still is not registered. We've added it for some existing CIPs; but I would refrain to add it to any new ones..
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'm assuming there will be one, though.
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've been assuming for a couple of months too now 🙃
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.
just noting for cross-reference: there is soon going to be one 🙂 #456
Thanks, I'll fix those things. I can add the "Path to Active" although I'm not sure what I'd say. By its nature this CIP is Active in virtue of us endorsing it, so... what? |
If sticking to the guidelines as I understand them, maybe something like (edited to your satisfaction): Acceptance CriteriaAgreement upon this CIP by the Plutus team is confirmation of its acceptance. Implementation PlanImplementation of the CIP proceeds as a matter of course after the Plutus team agrees upon it. If it sounds dumb to have these tautological paths-to-active then maybe @KtorZ we could allow for its omission in certain cases... 🤔 |
@michaelpj since you were also editing CIP-0001 when the longer titles were assigned to these 2 sections maybe you could please also contribute your opinion about the latter part of #439. |
I'd be happy to see just this as a path to active :) |
I've addressed the comments, and removed the multiple categories. I've attempted to say something sensible about what to do in cases where things fall in the overlap, but we can fix it up later if it proves problematic I guess. |
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 @michaelpj - this all looks both compliant & well explained to me.
@KtorZ are we strictly requiring that the H1 document titles contain the CIP-XXXX identifier as well?
- I ask mainly because they've been edited into CIPs 2 through 4 (and I followed suit with 13) in the course of Update older CIPs for revised CIP-0001 #389.
- It seems to me this could be left up to the author considering the number will be in the published URL and also just above in the header.
Sorry for another nitpick from the format nanny, but also trying to make sure whatever CI we might invent to normalise the presentation doesn't have to be more like an AI. 🤓
Perhaps not. I am on the fence with that. It reads better on Github, but it's currently redundant on the website (see e.g. https://cips.cardano.org/cips/cip1/). Plus, there's the "problem of section heading" that you outlined in #439 ... which makes me think we might just consider dropping it and bumping all section headers to |
Section headers as H1's is against the SEO point of view in which H1 is synonymous with the page title... while H2's for section headers is perfect. The problem with the current web site rendering is the duplicated H1 title. So the problem is solved by eliminating the H1 title in the CIP text (allowing the web script to generate the H1 title) but leaving all the section headers as H2 (or making them that way). @michaelpj any further formatting discussions can go back to #439 and if @KtorZ agrees with the above then your document formatting is fine the way it is... since if there's an extra H1 in the GitHub text then it'll easy to filter out in the web site generator. |
Okay, sounds like you're happy with this overall then? |
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.
Yes 😎
* Update CIP-35 to be more in line with the new CIP-001 * Update metadata for CIP-(31-33) * Address comments
This updates CIP-35 to be in line with the new CIP-1.
The main changes are:
There's one very questionable thing that I've done, which is to assert that CIPs in the crossover between the Ledger and Plutus should be in both categories. I just went ahead and wrote that here, but I defer to the resolution of #433 and I'll change this depending on what gets decided there.