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 EIP-6366: Move to Review #6770

Merged
merged 10 commits into from
Oct 24, 2023

Conversation

chiro-hiro
Copy link
Contributor

@chiro-hiro chiro-hiro commented Mar 24, 2023

When opening a pull request to submit a new EIP, please use the suggested template: https://github.com/ethereum/EIPs/blob/master/eip-template.md

We have a GitHub bot that automatically merges some PRs. It will merge yours immediately if certain criteria are met:

  • The PR edits only existing draft PRs.
  • The build passes.
  • Your GitHub username or email address is listed in the 'author' header of all affected PRs, inside .
  • If matching on email address, the email address is the one publicly listed on your GitHub profile.

@github-actions github-actions bot added c-update Modifies an existing proposal s-draft This EIP is a Draft t-erc labels Mar 24, 2023
@eth-bot eth-bot changed the title Update eip-6366 following the suggestion from Victor Update EIP-6366: Update eip-6366 following the suggestion from Victor Mar 24, 2023
@eth-bot
Copy link
Collaborator

eth-bot commented Mar 24, 2023

✅ All reviewers have approved.

vdusart
vdusart previously approved these changes Mar 24, 2023
@chiro-hiro chiro-hiro marked this pull request as ready for review March 24, 2023 10:23
@chiro-hiro chiro-hiro requested a review from vdusart March 24, 2023 10:23
@chiro-hiro
Copy link
Contributor Author

@eth-bot rerun

vdusart
vdusart previously approved these changes Mar 24, 2023
@github-actions github-actions bot added c-status Changes a proposal's status s-review This EIP is in Review and removed c-update Modifies an existing proposal s-draft This EIP is a Draft labels Mar 24, 2023
@eth-bot eth-bot changed the title Update EIP-6366: Update eip-6366 following the suggestion from Victor Update EIP-6366: Move to Review Mar 24, 2023
@eth-bot eth-bot added the e-review Waiting on editor to review label Mar 24, 2023
@github-actions github-actions bot added the w-ci Waiting on CI to pass label Mar 24, 2023
Copy link
Contributor

@SamWilsn SamWilsn left a comment

Choose a reason for hiding this comment

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

This proposal needs to have a fleshed out rationale section before it can move into review.


Line 166 has:

SHOULD NOT expected IEIP6366Error interface was implemented.

I'm not sure what that means.

EIPS/eip-6366.md Outdated Show resolved Hide resolved
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@chiro-hiro
Copy link
Contributor Author

This proposal needs to have a fleshed out rationale section before it can move into review.

Line 166 has:

SHOULD NOT expected IEIP6366Error interface was implemented.

I'm not sure what that means.

This interface is optional.

@SamWilsn
Copy link
Contributor

This interface is optional.

Don't tell me, put it in the EIP 😛

Perhaps:

Compatible tokens MAY implement `IEIP6366Error` as defined below:

@SamWilsn
Copy link
Contributor

Looks like line 166 still needs to be updated, and I can't modify your branch. Please update the line, and we can move this proposal to review.

@chiro-hiro
Copy link
Contributor Author

@eth-bot rerun

@chiro-hiro
Copy link
Contributor Author

Hi @SamWilsn, I have updated this EIP following your request and also link the metadata interface to EIP-6617

@SamWilsn
Copy link
Contributor

Looks like line 166 is still the old text:

SHOULD NOT expected IEIP6366Error interface was implemented.


Is ERC-6617 ready to go to review as well? You'll need to open a PR for that too.

@chiro-hiro
Copy link
Contributor Author

chiro-hiro commented Sep 22, 2023

Hi @SamWilsn, sorry! I pushed to wrong branch. Now everything is updated.

Is ERC-6617 ready to go to review as well? You'll need to open a PR for that too.

Yes, ERC-6617 was moved to review.

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Sep 22, 2023
EIPS/eip-6366.md Outdated Show resolved Hide resolved
EIPS/eip-6366.md Show resolved Hide resolved
chiro-hiro and others added 2 commits October 3, 2023 10:41
Co-authored-by: Victor Dusart <43795504+vdusart@users.noreply.github.com>
Co-authored-by: Victor Dusart <43795504+vdusart@users.noreply.github.com>
@github-actions
Copy link

The commit fe2a963 (as a parent of 8dc8ac0) contains errors.
Please inspect the Run Summary for details.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Oct 10, 2023
@chiro-hiro
Copy link
Contributor Author

Hi @SamWilsn,

I updated EIP-6366

  • Moved Metadata to EIP-6617
  • Remove example files
  • Fix the description of Error Interface

CC: @vdusart

@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Oct 10, 2023
@eth-bot eth-bot enabled auto-merge (squash) October 24, 2023 00:55
Copy link
Collaborator

@eth-bot eth-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...

@eth-bot eth-bot merged commit b8e399a into ethereum:master Oct 24, 2023
11 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-status Changes a proposal's status e-review Waiting on editor to review s-review This EIP is in Review t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants