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

1193: Final edits in response to feedback #2695

Merged
merged 3 commits into from Jun 4, 2020

Conversation

rekmarks
Copy link
Contributor

@rekmarks rekmarks commented Jun 4, 2020

Link to file: https://github.com/rekmarks/EIPs/blob/1193-final-draft/EIPS/eip-1193.md

  • Normative
    • Update Events specification
      • Change recommendation to extend EventEmitter from normative to non-normative
        • In recognition that the EventEmitter API, while stable is growing over time
    • Remove leftover index type on one interface
      • This should have already been done
    • Remove all normative references to "Client"
  • Non-normative
    • Trim "Summary" section
    • Trim message event "Subscription" subsection in API documentation
    • Remove all deprecated methods and events from the API documentation
      • They were only specified in the API documentation
    • Remove "Availability" section from specification (non-normative section)
    • Various minor fixups

@rekmarks rekmarks mentioned this pull request Jun 4, 2020
EIPS/eip-1193.md Outdated
@@ -15,12 +15,10 @@ requires: 155, 695

This EIP formalizes a JavaScript Ethereum Provider API for consistency across clients and applications.

The Provider's interface is designed to be minimal, preferring that features are introduced in the API layer (see e.g. [`eth_requestAccounts`](https://eips.ethereum.org/EIPS/eip-1102)), and agnostic of transport and RPC protocols.
The Provider's interface is designed to be minimal and agnostic of transport and RPC protocols.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link
Contributor

Choose a reason for hiding this comment

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

My intention with these words were for implementers to keep away from extending and adding their own methods and properties to the provider object as it just causes confusion and fragmentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The new text will be as follows, and I do think that it conveys that new functionality can be added via new RPC methods.

The Provider's interface is designed to be minimal, event-driven, and agnostic of transport and RPC protocols.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added back updated language reflecting goals in ffe92ce

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

@ryanio ryanio left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

@rekmarks rekmarks marked this pull request as ready for review June 4, 2020 19:31
@eip-automerger eip-automerger merged commit c685eca into ethereum:master Jun 4, 2020
@rekmarks rekmarks deleted the 1193-final-draft branch June 4, 2020 19:36
pizzarob pushed a commit to pizzarob/EIPs that referenced this pull request Jun 12, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
tkstanczak pushed a commit to tkstanczak/EIPs that referenced this pull request Nov 7, 2020
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
Arachnid pushed a commit to Arachnid/EIPs that referenced this pull request Mar 6, 2021
Hi, I'm a bot! This change was automatically merged because:

 - It only modifies existing Draft or Last Call EIP(s)
 - The PR was approved or written by at least one author of each modified EIP
 - The build is passing
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.

None yet

3 participants