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 Hello World template [Fixes #234] #235

Merged
merged 4 commits into from
Jun 8, 2020
Merged

Conversation

samajammin
Copy link
Collaborator

Description of the Change

Refactored the Hello World template (primarily the README) based on the extensive feedback we received from a group of Ethereum developers.

Alternate Designs

See #234

Benefits

See #234

Possible Drawbacks

Users must go to the project files to read the code vs. the README, however, that's intended & based on the feedback we received will improve UX.

Verification Process

Ran the project locally.

Github Issues

#234

@samajammin
Copy link
Collaborator Author

samajammin commented May 27, 2020

I welcome input from anyone on this!

I highly recommend running the project locally & using the template. It's much easier to grok the new instructions & flow vs. reading the file changes on Github.

this.instance = this.Contract.address ? contract_interface.at(this.Contract.address) : { message: () => {} };
};
```
Dapps typically use a [JavaScript convenience libraries](https://ethereum.org/developers/#frontend-javascript-apis) that provide an API to make integrations with smart contract easier for developers. In this project, you'll be using [web3.js](https://web3js.readthedocs.io/en/v1.2.8/).
Copy link
Contributor

Choose a reason for hiding this comment

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

These are typically called APIs or SDKs, any reason you chose "convenience library"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I just noticed that's the terminology MetaMask uses: https://docs.metamask.io/guide/getting-started.html#choosing-a-convenience-library

Figured it'd be good to align with them given they have perhaps the most widely used Ethereum wallet among devs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suppose my personal bias is also that the term "SDK" feels a bit heavy-weight to describe ethers.js or web3.js. They are JavaScript libraries that provide APIs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Open to changing this if there's evidence of those terms being consistently used elsewhere.

Copy link
Contributor

@ChrisChinchilla ChrisChinchilla left a comment

Choose a reason for hiding this comment

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

Few small things on grammar, and some questions. I'd also like to try running through it myself to see if it makes sense when following the application code.

And… I think it makes sense to re-record the video based around this, so I can test that it makes sense as I do that.

Co-authored-by: Chris Chinchilla <chris.ward@ethereum.org>
@samajammin
Copy link
Collaborator Author

Thanks for the review @ChrisChinchilla! I incorporated all your suggested changes.

I'd also like to try running through it myself to see if it makes sense when following the application code.

Sure thing, that's what I recommended 😄 .

And… I think it makes sense to re-record the video based around this, so I can test that it makes sense as I do that.

Perhaps but how long does a video take to record? We potentially have a series of upcoming changes to Ethereum Studio (see all the open issues), I would like to avoid you having to re-record a bunch of times. I suggest we hold off on that for now.

@samajammin
Copy link
Collaborator Author

Thanks for the review @javier-tarazaga. @ChrisChinchilla I'll wait for another review from you before merging anything.

Copy link
Contributor

@ChrisChinchilla ChrisChinchilla left a comment

Choose a reason for hiding this comment

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

Looks good to me now!

@samajammin samajammin merged commit 364730a into master Jun 8, 2020
@samajammin samajammin deleted the hello-world-v2 branch June 8, 2020 17:18
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