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-0010 | Validation by GitHub workflow #837

Merged
merged 6 commits into from
Jul 14, 2024

Conversation

Godspeed-exe
Copy link
Contributor

Hey guys,

I took some inspiration from the Cardano Token Registry Github repo and implemented a Github workflow which:

  • on every PR will run automatically
  • will validate the structure of the JSON schema (which I simplified as well).

You can check out how this works on my fork:

Let me know if you have any questions / remarks / suggestions!

@rphair rphair changed the title CIP-0010 - Automated validation Github workflow for new entries CIP-0010 | Validation by GitHub workflow Jun 6, 2024
@rphair
Copy link
Collaborator

rphair commented Jun 6, 2024

thanks for your effort @Godspeed-exe - we can talk about this potential change at the next CIP meeting (I've put it on the agenda: https://hackmd.io/@cip-editors/90).

Processing these PRs is a low bandwidth task for editors and the textural regularity of entries does tend to make errors easily spotted by eye. My main discussion point for other editors would therefore be, "Do we need this?"

@Godspeed-exe
Copy link
Contributor Author

No worries @rphair

Recent events would argue that some of these errors do slip by occasionally and go unnoticed for months (very subtle mistakes like metadautum) and having this automated check would severely decrease chances of this happening again.

I'm aiming / hoping to create many more standards in the upcoming future so I do expect this to be growing. From Enterprise Technology side we stress for registration and documentation of these standards. Good documentation is (as we know) key for growth and adoption.

I'm also building a component which would make pushing metadata to Cardano way more accessible for SME's and this is also syncing / checking with CIP-0010 for registration and preferably also using the JSON schema validators - otherwise people will be pushing useless rubbish.

Thanks for the consideration!

@Godspeed-exe
Copy link
Contributor Author

Godspeed-exe commented Jun 6, 2024 via email

@Crypto2099
Copy link
Collaborator

I like this idea, particularly for CIPs that use JSON or similar for "registries" to ensure that all changes are already 'correct' without the need for manual human intervention.

@rphair rphair requested a review from Ryun1 June 10, 2024 15:44
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.

@Godspeed-exe we just discussed this at the CIP editors' meeting and for a couple reasons I wanted to retract my earlier lukewarm comment:

  • this material sets a precedent & an initial test case for validating CIP changes in general... there are other validations like file & directory names which checks for in advance would help authors & editors avoid inappropriate merges.
  • other registries besides CIP-0010 will have more complicated schema & syntax and so it would help to test on the much simpler case of CIP-0010 first.
  • @Crypto2099 pointed out we'll also have Internet standardised CBOR tags in the registry, so integration with outside sites may be an issue going forward and we will need automation & validation for these.

So after a preliminary test & verification in an editor's fork is done, I would be in favour of merging this when we have an additional verification that it will perform as expected. 😎

@Godspeed-exe
Copy link
Contributor Author

@rphair thanks for the update, let me know if you have any questions or if anything needs to be improved! 💪

Copy link
Collaborator

@Crypto2099 Crypto2099 left a comment

Choose a reason for hiding this comment

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

All of these changes look good to me and I think this one is ready for a real-world test.

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.

Willing to give this a try now that schema question has been resolved (#837 (comment)).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants