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

Update ERC-7208: Move to Review #568

Merged
merged 48 commits into from
Sep 25, 2024
Merged

Conversation

galimba
Copy link
Contributor

@galimba galimba commented Aug 1, 2024

We request to push ERC-7208 to Review and Last Call soon, after validating with the community actively using this architecture.

Additionally, we provide an educational implementation that acts as a reference to the interoperability capabilities of this approach.

pash7ka and others added 29 commits December 4, 2023 14:55
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
erc-7208 update: removed todo

erc-7208 update: update metadata provider interface

erc-7208 update: add mutation types
ERC7208:update Abstract

ERC7208:update Motivation

ERC7208:update interfaces

ERC7208:update rationale

ERC7208:update grammar

ERC7208:update assets

ERC7208:update reference implementation

ERC7208:update removed erc721 dependency

ERC7208:update fix comment

ERC7208:update fixes to interfaces

ERC7208:update fixes to interfaces

ERC7208:update fixes to interfaces

ERC7208:update fixes to rationale

ERC7208:update fixes to implementation
erc7208 update: grammar fix

erc7208 update: grammar fix
eip-review-bot
eip-review-bot previously approved these changes Aug 28, 2024
Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

auto-merge was automatically disabled August 28, 2024 18:47

Head branch was pushed to by a user without write access

ERCS/erc-7208.md Outdated
@@ -271,6 +271,8 @@ We present an **educational example** implementation showcasing two types of tok
**This example has not been audited and should not be used in production environments.**


See [Minimalistic Data Index](https://github.com/NexeraProtocol/Minimalistic-Data-Index) implementation.
Copy link
Collaborator

Choose a reason for hiding this comment

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

The linter is correct here. For the reasons enumerated here, please remove this external link.

Copy link

@pash7ka pash7ka Sep 5, 2024

Choose a reason for hiding this comment

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

Hi @SamWilsn ,
I've removed the external link replacing it with the link to README.md, as you've suggested in the comment above.
But now the HTML prufer fails with

For the Links > Internal check, the following failures were found:
* At ./_site/EIPS/eip-7208.html:526:
  internally linking to ../assets/erc-7208/contracts/README.md, which does not exist
HTML-Proofer found 1 failure!

which was the reason we've tried with external repo.
Do you have any ideas how to fix it?

@github-actions github-actions bot removed the w-ci label Sep 5, 2024
Copy link

github-actions bot commented Sep 5, 2024

The commit f9c65e8 (as a parent of 01887ab) contains errors.
Please inspect the Run Summary for details.

ERCS/erc-7208.md Outdated
@@ -271,6 +271,8 @@ We present an **educational example** implementation showcasing two types of tok
**This example has not been audited and should not be used in production environments.**


See [contracts](../assets/erc-7208/contracts/README.md)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Because of a quirk of our rendering system, this needs to be:

Suggested change
See [contracts](../assets/erc-7208/contracts/README.md)
See [contracts](../assets/eip-7208/contracts/README.md)

Copy link

Choose a reason for hiding this comment

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

Done! Thank you!

@github-actions github-actions bot removed the w-ci label Sep 17, 2024
@eip-review-bot eip-review-bot enabled auto-merge (squash) September 25, 2024 18:55
Copy link
Collaborator

@eip-review-bot eip-review-bot left a comment

Choose a reason for hiding this comment

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

All Reviewers Have Approved; Performing Automatic Merge...

@eip-review-bot eip-review-bot merged commit 34794c3 into ethereum:master Sep 25, 2024
11 checks passed
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