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-20: Renovate #6038

Closed
wants to merge 11 commits into from
Closed

Update EIP-20: Renovate #6038

wants to merge 11 commits into from

Conversation

Pandapip1
Copy link
Member

@Pandapip1 Pandapip1 commented Nov 23, 2022

WIP. Likely to be contentious.

@Pandapip1 Pandapip1 added the e-consensus Waiting on editor consensus label Nov 23, 2022
@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Nov 23, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Nov 23, 2022

File EIPS/eip-20.md

Requires 2 more reviewers from @axic, @gcolvin, @lightclient, @SamWilsn

@Pandapip1 Pandapip1 marked this pull request as ready for review November 23, 2022 20:17
@Pandapip1 Pandapip1 changed the title Update EIP-20: Renovate to fit current EIP-1 recommendations Update EIP-20: Renovate to fit most current EIP-1 recommendations Nov 23, 2022
@lightclient
Copy link
Member

Strongly against. We've never rewritten EIPs to fit the "new recommendations" and I don't think we should start.

@Pandapip1
Copy link
Member Author

Pandapip1 commented Dec 6, 2022

We've never rewritten EIPs to fit the "new recommendations" and I don't think we should start.

We have modified EIPs to fit new recommendations before (#5055). It's true that we've never rewritten them, though.

@SamWilsn
Copy link
Contributor

After today's EIPIP, I think the consensus is that this PR changes more than the absolute minimum to comply with the current EIP-1 guidelines. For example, moving Simple Summary to description: is probably fine, while changing the Solidity version is too much

@Pandapip1
Copy link
Member Author

Pandapip1 commented Dec 14, 2022

That seems reasonable. I'll revert the extra (unneeded) changes.

@github-actions github-actions bot added the w-ci Waiting on CI to pass label Jan 18, 2023
@github-actions
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the w-stale Waiting on activity label Feb 23, 2023
@Pandapip1
Copy link
Member Author

@SamWilsn mind re-reviewing?

@Pandapip1
Copy link
Member Author

@eth-bot rerun

EIPS/eip-20.md Outdated Show resolved Hide resolved
EIPS/eip-20.md Outdated Show resolved Hide resolved
EIPS/eip-20.md Show resolved Hide resolved
EIPS/eip-20.md Outdated
Comment on lines 147 to 153
### The approve and transferFrom flow

`approve` and `transferFrom` can be used in conjunction with one another to allow smart contracts to take an action if and only if a transfer succeeds. This enables use cases such as decentralized exchanges, which can trustlessly swap one token for another.

## Implementation
### Decimals is optional

There are already plenty of ERC20-compliant tokens deployed on the Ethereum network.
Different implementations have been written by various teams that have different trade-offs: from gas saving to improved security.
Some tokens might wish to use a base unit other than $10^{-18}$, and sometimes it doesn't make sense to have a fraction of a token. However, not every token might reasonably need to specify `decimals`. Thus, `decimals` is optional.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should invent content that wasn't there in the original proposal.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a non-normative required section. If I were the person writing ERC-20 and decided to include this field, that would be why. If the original authors consent/agree with the motivation provided then I see nothing wrong with this.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is still more than is required to comply with current guidelines, and I don't think this qualifies as errata.

Pandapip1 and others added 2 commits March 3, 2023 13:49
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@github-actions github-actions bot removed the w-ci Waiting on CI to pass label Mar 3, 2023
Co-authored-by: Sam Wilson <57262657+SamWilsn@users.noreply.github.com>
@github-actions github-actions bot added w-ci Waiting on CI to pass and removed w-stale Waiting on activity labels Mar 3, 2023
@github-actions
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the w-stale Waiting on activity label Mar 27, 2023
@Pandapip1
Copy link
Member Author

Hey @SamWilsn, mind checking out the changes now?

@github-actions github-actions bot removed the w-stale Waiting on activity label Mar 28, 2023
@github-actions
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the w-stale Waiting on activity label Apr 12, 2023
@eth-bot eth-bot changed the title Update EIP-20: Renovate to fit most current EIP-1 recommendations Update EIP-20: Move to Final Apr 12, 2023
@github-actions github-actions bot added w-ci Waiting on CI to pass and removed w-ci Waiting on CI to pass labels Apr 12, 2023
@Pandapip1
Copy link
Member Author

Oh lol that's a funny eip-review-bot bug. I'll get that fixed.

@github-actions github-actions bot removed the w-stale Waiting on activity label Apr 13, 2023
@Pandapip1 Pandapip1 changed the title Update EIP-20: Move to Final Update EIP-20: Renovate Apr 25, 2023
@Pandapip1 Pandapip1 requested a review from SamWilsn April 25, 2023 13:04
@github-actions
Copy link

The commit 9bfa57f (as a parent of 1a0a905) contains errors.
Please inspect the Run Summary for details.

@github-actions
Copy link

There has been no activity on this pull request for 2 weeks. It will be closed after 3 months of inactivity. If you would like to move this PR forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the w-stale Waiting on activity label May 10, 2023
@github-actions
Copy link

This pull request was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

@github-actions github-actions bot closed this Jun 21, 2023
@Pandapip1 Pandapip1 deleted the Pandapip1-eip-20-renovate branch October 27, 2023 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-update Modifies an existing proposal e-consensus Waiting on editor consensus s-final This EIP is Final t-erc w-ci Waiting on CI to pass w-stale Waiting on activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants