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-3475: Reference Implementation Typo Fixes #5547

Merged
merged 4 commits into from Sep 12, 2022

Conversation

Pandapip1
Copy link
Member

Fixes two typos found in the EIP-3475 reference implementation.

@Pandapip1 Pandapip1 added the a-review Waiting on author to review label Aug 27, 2022
@eth-bot
Copy link
Collaborator

eth-bot commented Aug 27, 2022

A critical exception has occurred:
Message: pr 5547 is already merged; quitting
(cc @alita-moore, @mryalamanchi)

@Pandapip1 Pandapip1 changed the title Fix EIP-3475: Reference Implementation Typos EIP-3475: Reference Implementation Typo Fixes Aug 28, 2022
@Pandapip1 Pandapip1 changed the title EIP-3475: Reference Implementation Typo Fixes Update EIP-3475: Reference Implementation Typo Fixes Aug 28, 2022
@Pandapip1 Pandapip1 removed the a-review Waiting on author to review label Sep 2, 2022
@Pandapip1 Pandapip1 added this to the Manual Merge Queue milestone Sep 2, 2022
@SamWilsn
Copy link
Contributor

SamWilsn commented Sep 12, 2022

The Specification section uses _amount in the Redeem event, so I'm inclined to not make this change.


Also _amount is used in the Transaction struct in the Rationale section 🤷

@SamWilsn SamWilsn mentioned this pull request Sep 12, 2022
@Pandapip1
Copy link
Member Author

The Specification section uses _amount in the Redeem event, so I'm inclined to not make this change.

Not sure what that has to do with the current change here - if anything, this fixes that.

Also _amount is used in the Transaction struct in the Rationale section 🤷

Again, I'm pretty sure that you're confusing the lines that changed. This PR changes amount to _amount, not vice versa.

@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Sep 12, 2022
@Pandapip1 Pandapip1 added a-review Waiting on author to review and removed c-update Modifies an existing proposal t-erc s-final This EIP is Final labels Sep 12, 2022
@Pandapip1 Pandapip1 removed this from the Manual Merge Queue milestone Sep 12, 2022
@github-actions github-actions bot added c-update Modifies an existing proposal s-final This EIP is Final t-erc labels Sep 12, 2022
@eth-bot eth-bot enabled auto-merge (squash) September 12, 2022 12:42
@eth-bot eth-bot merged commit 0c5d19b into master Sep 12, 2022
@eth-bot eth-bot deleted the Pandapip1-3475-typos branch September 12, 2022 12:42
@dhruvmalik007
Copy link
Contributor

#5547 (comment)

thanks for changing this and my bad that this issue got through the check, it was named initially amount but we changed to the _amount just to differentiate it as the private variable..

@SamWilsn
Copy link
Contributor

Again, I'm pretty sure that you're confusing the lines that changed. This PR changes amount to _amount, not vice versa.

That's what I get for reviewing in the wee hours of the morning 🤣

nachomazzara pushed a commit to nachomazzara/EIPs that referenced this pull request Jan 13, 2023
* Fix EIP-3475: Reference Implementation Typos

* Make minor change to satisfy @eth-bot
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-review Waiting on author to review c-update Modifies an existing proposal s-final This EIP is Final t-erc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants