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

LAST CALL - READY to add eip155 #2

Merged
merged 12 commits into from
Apr 1, 2022
Merged

LAST CALL - READY to add eip155 #2

merged 12 commits into from
Apr 1, 2022

Conversation

bumblefudge
Copy link
Collaborator

@bumblefudge bumblefudge commented Jan 31, 2022

Open question:

  • should the JSON-RPC specs be incorporated as an ## CAIP25 and/or ## CAIP27 section?
    • on 2/10 call, sections purely for examples might be useful but overly verbose- might make sense to generalize CAIP25 and/or 27 to define better how those sections get added to namespaces
  • does the intro need some work? I feel like I'm probably glossing over many subtleties of the difference between evm and eip155 😅

Copy link
Member

@ligi ligi left a comment

Choose a reason for hiding this comment

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

Initial thoughts as comments
Also guess when adding more namespaces in the PR we might see more clearly where duplications for for certain sections would happen.

eip155.md Outdated Show resolved Hide resolved
eip155.md Outdated Show resolved Hide resolved
eip155.md Outdated Show resolved Hide resolved
@bumblefudge bumblefudge requested a review from ligi February 1, 2022 22:39
Copy link
Member

@ligi ligi left a comment

Choose a reason for hiding this comment

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

Just a typo - but think it is ready to merge soon - thanks for your work!

image

eip155.md Outdated Show resolved Hide resolved
eip155.md Outdated Show resolved Hide resolved
Bumblefudge and others added 2 commits March 9, 2022 20:35
Co-authored-by: ligi <ligi@ligi.de>
Co-authored-by: ligi <ligi@ligi.de>
@bumblefudge bumblefudge changed the title to add eip155 LAST CALL 0 to add eip155 Mar 10, 2022
@bumblefudge bumblefudge changed the title LAST CALL 0 to add eip155 LAST CALL - READY to add eip155 Mar 10, 2022
@bumblefudge
Copy link
Collaborator Author

As per consensus at last meeting, I've restructured the .md as a folder containing distinct markdown files for CAIPs. I took the liberty of giving a more human-readable name (i.e., "assets" rather than "CAIP-20/21", because I would imagine that some namespaces have to slice these things differently, as happened to me with the polkadot example, where the "addresses.md" file might not require CAIP-2, for example-- this slight decoupling of namespace markdown files from CAIPs allows them to map differently per namespace as needed.

@bumblefudge bumblefudge requested a review from oed March 24, 2022 14:10
Copy link
Contributor

@oed oed left a comment

Choose a reason for hiding this comment

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

Looks good. The only thing I would change is the name of the files, e.g. eip155/caip10.md instead of eip155/addresses.md. Would also be nice to have links to these documents from the eip155/README.md.

@oed
Copy link
Contributor

oed commented Mar 25, 2022

Oh, I see your comment now. I still think caip10.md would be better than addresses.md because it's clear what it refers to. And it will be easier to see which CAIPs a namespace supports.

@bumblefudge
Copy link
Collaborator Author

That last point won me over, caip-2.md it is! 😄

As for readmes:

  • I think when viewed in Github or, say, VSC, the file explorer will function as a TOC, non?
  • Once I'm done building out v1 of /namespaces/ i'll start looking at a way to automate github pages using the jekyll-style YAML frontmatter... surely there's a way to autogenerate TOCs at the root and per-namespace level?

Copy link
Contributor

@oed oed left a comment

Choose a reason for hiding this comment

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

Structure lgtm, some fixes needed to markdown formatting.

eip155/README.md Show resolved Hide resolved
eip155/README.md Show resolved Hide resolved
@ChainAgnostic ChainAgnostic deleted a comment from oed Mar 27, 2022
eip155/README.md Show resolved Hide resolved
@bumblefudge
Copy link
Collaborator Author

2 approvals, should we 🚢 it, @ligi ?

@ligi ligi merged commit d8e80e2 into main Apr 1, 2022
@bumblefudge bumblefudge mentioned this pull request Jan 20, 2023
bumblefudge pushed a commit that referenced this pull request May 23, 2024
* feat: Adds CAIP-122 standard for XRP Ledger.

* fix: Add some clarifications and follow the ripple-keypairs implementations for signing and verification

* fix: Branch was renamed from master to main

* fix: Remove double
bumblefudge pushed a commit that referenced this pull request Aug 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants