Skip to content
This repository has been archived by the owner on Feb 9, 2023. It is now read-only.

Set EIP number immediately before merging new EIP pull requests #64

Open
SamWilsn opened this issue Apr 20, 2022 · 8 comments
Open

Set EIP number immediately before merging new EIP pull requests #64

SamWilsn opened this issue Apr 20, 2022 · 8 comments
Labels
enhancement New feature or request priority: medium

Comments

@SamWilsn
Copy link

SamWilsn commented Apr 20, 2022

Originally from #55

Immediately before merging (IMO, ideally in the merge commit itself) set the EIP number in the preamble and rename the markdown file. How that number is determined probably needs to be discussed here.

@JEAlfonsoP
Copy link
Contributor

I will work on it.

@Pandapip1
Copy link
Member

I suggest that authors just send a comment of the form:

@eth-bot assign XXXX

where XXXX is the EIP number. This will perform the rename and other formatting on the spot.

If no number is present when it would be merged, then the PR number can be used, meaning that the comment can be used for cases of number gaming.

@SamWilsn
Copy link
Author

What's the advantage of assigning the number before merging? @MicahZoltu has mentioned he'd like to avoid assigning EIP numbers until the EIP is merged, and I'd tend to agree with that.

I was sorta envisioning a bors-style bot that creates the merge commit and pushes it to the default branch.

@JEAlfonsoP
Copy link
Contributor

This one confused me. am I wrong if I said that only editors assign EIP number (manually ?) ?

@MicahZoltu
Copy link
Contributor

Currently editors manually assign EIP numbers. This issue proposes we change that, but I am a bit worried about people gaming the system if it becomes fully automated.

@Pandapip1
Copy link
Member

See my suggestion at #64 (comment)

This would prevent number gaming by allowing EIP editors to manually change the EIP number.

@JEAlfonsoP
Copy link
Contributor

I suggest to talk about it during the next call and agree in one action (if any)

@SamWilsn
Copy link
Author

Ideally I'd like to avoid assigning an EIP number until the PR is merged, so my preference would be for a comment allowing EIP editors to override the number without changing the file itself. Something like:

@eip-bot number 1234

That said, I think an easier to implement solution would be to only auto-assign a number on merge if a number isn't already set.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request priority: medium
Projects
None yet
Development

No branches or pull requests

4 participants