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.

Sign up for GitHub

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

Jump to bottom

Create EIP-4804 #4995

Merged
merged 20 commits into from
May 22, 2022
Merged

Create EIP-4804 #4995

merged 20 commits into from
May 22, 2022

Conversation

qizhou
Copy link
Contributor

@qizhou qizhou commented Apr 13, 2022

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.

@eth-bot
Copy link
Collaborator

eth-bot commented Apr 13, 2022 •
edited
Loading

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

(fail) eip-4804.md

classification
newEIPFile
  • File with name EIPS/eip-4804.md is new and new files must be reviewed
  • This PR requires review from one of [@lightclient, @axic]

SamWilsn
SamWilsn requested changes Apr 14, 2022
View reviewed changes
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Show resolved Hide resolved
qizhou and others added 10 commits April 14, 2022 15:58
@qizhou @SamWilsn
Update EIPS/eip-4804.md
5f34c72 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
689a18e 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
68761e9 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
7256a66 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
32f99cf 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou
Update eip-4804.md
7ee6c6f 
- Change ## Example to ### Example
- Move "Simple summary" to the description
- Add Rationale part
- Update authors
- Add the part about how to resolve address from NS
@qizhou
Update eip-4804.md
a00ec76
@qizhou
Polish name service suffix
945af97
@qizhou
Update eip-4804.md
2f64549
@qizhou
Update eip-4804.md
727095a
SamWilsn
SamWilsn reviewed Apr 22, 2022
View reviewed changes
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Show resolved Hide resolved
SamWilsn
SamWilsn reviewed Apr 30, 2022
View reviewed changes
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Show resolved Hide resolved
qizhou and others added 3 commits May 2, 2022 11:09
@qizhou @SamWilsn
Update EIPS/eip-4804.md
c3e61f4 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
643e90b 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@qizhou @SamWilsn
Update EIPS/eip-4804.md
7b4e121 
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@poojaranjan poojaranjan mentioned this pull request May 3, 2022
Closed
lightclient
lightclient previously requested changes May 3, 2022
View reviewed changes
EIPS/eip-4804.md Show resolved Hide resolved
EIPS/eip-4804.md Outdated Show resolved Hide resolved
EIPS/eip-4804.md Show resolved Hide resolved
Pandapip1
Pandapip1 reviewed May 13, 2022
View reviewed changes
EIPS/eip-4804.md Outdated Show resolved Hide resolved
@qizhou @Pandapip1
Update EIPS/eip-4804.md
0343b9f 
Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
@MicahZoltu
Copy link
Contributor

Hmm, Sam's approval should have been sufficient...

@MicahZoltu MicahZoltu closed this May 21, 2022
@MicahZoltu MicahZoltu reopened this May 21, 2022
@MicahZoltu MicahZoltu dismissed lightclient’s stale review May 21, 2022 04:30

Changes addressed.

@MicahZoltu MicahZoltu closed this May 21, 2022
@MicahZoltu MicahZoltu reopened this May 21, 2022
@MicahZoltu
Copy link
Contributor

I believe the issue was that lightclient had an outstanding change request review. I dismissed it, so this should auto-merge now.

@MicahZoltu
Copy link
Contributor

@SamWilsn @alita-moore Any idea why the bot is looking for lightclient/axic and not Sam? The branch has Sam in the ERC editor list, as does ethereum/EIPs/:master.

https://github.com/qizhou/EIPs/blob/0343b9f982d7a84ec121fff8180f8b28dc165ed8/.github/workflows/auto-merge-bot.yml#L21

EIPs/.github/workflows/auto-merge-bot.yml

Line 21 in 124c9f7

ERC_EDITORS: "@lightclient,@axic,@SamWilsn"

@alita-moore
Copy link
Contributor

It's because it requires a review from an editor. Which in this case is axic or light client. There is not a rule that requires author approval on a new file. Although that is a good idea.

@MicahZoltu
Copy link
Contributor

MicahZoltu commented May 21, 2022 •
edited
Loading

@alita-moore Sam is an editor. I can't figure out why for this PR (and only this PR) the bot seems to think he isn't.

@MicahZoltu
Copy link
Contributor

Oh! It is because Sam is also an author, and you can't approve your own EiPs.

@alita-moore
Copy link
Contributor

It could also be because of the fact different authors for different EIP types.

MicahZoltu
MicahZoltu reviewed May 22, 2022
View reviewed changes
EIPS/eip-4804.md Outdated Show resolved Hide resolved
@MicahZoltu
Copy link
Contributor

Going to manually merge this since Sam approved (but he is an author) and I glanced over it (but I'm not an ERC reviewer). I feel like two half-qualified reviews equals one full review. 😬

@MicahZoltu MicahZoltu merged commit 49fc53b into ethereum:master May 22, 2022
nachomazzara pushed a commit to nachomazzara/EIPs that referenced this pull request Jan 13, 2023
@qizhou @SamWilsn
@Pandapip1 @MicahZoltu @nachomazzara
Create EIP-4804 (ethereum#4995)
8607d1a 
* Create eip-4804.md

* Update eip-4804.md

* Update eip-4804.md

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update eip-4804.md

- Change ## Example to ### Example
- Move "Simple summary" to the description
- Add Rationale part
- Update authors
- Add the part about how to resolve address from NS

* Update eip-4804.md

* Polish name service suffix

* Update eip-4804.md

* Update eip-4804.md

* Update eip-4804.md

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>

* Fix some typos

* Update EIPS/eip-4804.md

Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>

* Update EIPS/eip-4804.md

Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
Co-authored-by: Pandapip1 <45835846+Pandapip1@users.noreply.github.com>
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Pandapip1 Pandapip1 Pandapip1 left review comments

@MicahZoltu MicahZoltu MicahZoltu left review comments

@SamWilsn SamWilsn SamWilsn approved these changes

@lightclient lightclient lightclient left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@qizhou @eth-bot @MicahZoltu @alita-moore @lightclient @Pandapip1 @SamWilsn