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

Fix CIP information generation by removing "type" property. #1135

Closed

Conversation

fill-the-fill
Copy link
Collaborator

Checklist

  • I have read the How to Contribute.
  • I have run yarn build after adding my changes without getting any errors.

Updating documentation or Bugfix

Since according to CIP-0001, type property in header was quote:"confusing and not-too-useful" I don't believe we should use it in CIP information at the end of each CIP generated on Developer Portal. In the example bellow CIP-0002 doesn't have type property so its displaying null.

Currently on Master it looks like this:
Screenshot 2023-08-22 at 15 50 53

I suggest we make it look like this:
Screenshot 2023-08-22 at 15 52 16

@fill-the-fill fill-the-fill added bug Something isn't working script Improvements or additions of scripts labels Aug 22, 2023
@fill-the-fill fill-the-fill changed the title Fixed CIP information generation by removing "type" property. Fix CIP information generation by removing "type" property. Aug 22, 2023
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.

Another option would be to use:

  • Category if it's defined in the header, or
  • Type if not (i.e. the legacy CIP definition).

All CIPs originally had Type but then, following the updated CIP-0001, specified a Category instead. All CIPs had either one or the other as a requirement for being merged. Old CIPs that we've updated or will update soon (cardano-foundation/CIPs#389) will have Type removed and a Category assigned.

The old language that worked for Type would actually work well for Category preceding the name of the CIP: since the Category is also a modifier (Ledger CIP, Plutus CIP, etc.): https://github.com/cardano-foundation/CIPs/tree/master/CIP-0001#categories

After cardano-foundation/CIPs#389 is complete, all CIPs will have only a Category field and none of them will have a Type field.

@rphair
Copy link
Collaborator

rphair commented Sep 2, 2023

Just having a look at this again & think these other refinements would be easy to make if fully updating this code for the new CIP-0001 specification:

@fill-the-fill
Copy link
Collaborator Author

Just having a look at this again & think these other refinements would be easy to make if fully updating this code for the new CIP-0001 specification:

sounds like a good idea, let me update you here when I push a commit with the following changes.

@katomm
Copy link
Member

katomm commented Dec 4, 2023

Any updates on this? We seem not to move here.

@katomm katomm closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working script Improvements or additions of scripts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants