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

Create draft EIP-symbol #3014

Merged
merged 15 commits into from
Oct 3, 2020
Merged

Create draft EIP-symbol #3014

merged 15 commits into from
Oct 3, 2020

Conversation

PeterTheOne
Copy link
Contributor

@PeterTheOne PeterTheOne commented Sep 30, 2020

See discussion in #3012

EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

The suggestions from lightclient are required to merge, but overall the structure here looks good to go for draft.

EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
Comment on lines 46 to 50
## Rationale
<!-- The rationale fleshes out the specification by describing what motivated the design and why particular design decisions were made. It should describe alternate designs that were considered and related work, e.g. how the feature is supported in other languages. The rationale may also provide evidence of consensus within the community, and should discuss important objections or concerns raised during discussion. -->
(Similar to https://eips.ethereum.org/EIPS/eip-695 and symbol from https://eips.ethereum.org/EIPS/eip-20)

todo
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe mention why eth_symbol instead of eth_nativeCurrencySymbol?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I now mentioned it but I'm open for alternatives and more discussion.

EIPS/eip-symbol.md Outdated Show resolved Hide resolved
EIPS/eip-symbol.md Outdated Show resolved Hide resolved
PeterTheOne and others added 7 commits October 1, 2020 14:12
Co-authored-by: Micah Zoltu <micah@zoltu.net>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
EIPS/eip-3014.md Outdated Show resolved Hide resolved
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
EIPS/eip-3014.md Outdated Show resolved Hide resolved
EIPS/eip-3014.md Outdated Show resolved Hide resolved
EIPS/eip-3014.md Outdated Show resolved Hide resolved
PeterTheOne and others added 3 commits October 1, 2020 18:31
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Apologies @PeterTheOne, I made a mistake on my previous review. The relative links should also have *.md endings. Otherwise this looks good.

EIPS/eip-3014.md Outdated Show resolved Hide resolved
Co-authored-by: lightclient <14004106+lightclient@users.noreply.github.com>
@PeterTheOne
Copy link
Contributor Author

Apologies @PeterTheOne, I made a mistake on my previous review. The relative links should also have *.md endings. Otherwise this looks good.

@lightclient no problem, you have been very helpful! thnx

Copy link
Contributor

@MicahZoltu MicahZoltu left a comment

Choose a reason for hiding this comment

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

Let one more comment, but this is good to merge as draft.

@@ -0,0 +1,48 @@
---
eip: 3014
title: Create `eth_symbol` method for JSON-RPC
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
title: Create `eth_symbol` method for JSON-RPC
title: `eth_symbol` JSON-RPC method

Consider tightening up the title to be a bit more terse.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point #3018

@MicahZoltu MicahZoltu merged commit 59ab530 into ethereum:master Oct 3, 2020
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
Add `eth_symbol` method to the JSON-RPC that returns the symbol of the native coin of the network.
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
Add `eth_symbol` method to the JSON-RPC that returns the symbol of the native coin of the network.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants