docs: evmbin - Update Rust docs by ltfschoen · Pull Request #10658 · openethereum/parity-ethereum

ltfschoen · 2019-05-14T09:25:43Z

No description provided.

parity-cla-bot · 2019-05-14T09:25:46Z

It looks like this contributor signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

soc1c · 2019-05-14T11:24:33Z

+//!
+//! Build Parity Ethereum from source, then start it with:
+//! ```bash
+//! cd evmbin && cargo build --release && cd ..


something like

cargo build -p evmbin

and is much simpler

i've pushed a commit addressing your comment to simplify it

sorry, i've pushed a commit fixing it now so it's:

//! ```bash //! cargo build -p evmbin --release //! ./target/release/parity-evm --help //! ```

Is that ok @soc1c ?

ltfschoen · 2019-05-14T17:04:15Z

Credit to @tomusdrw who explained evmbin too me so I was able to add some of the comments

dvdplm · 2019-05-15T07:21:24Z

+//!
+//! ## Usage
+//!
+//! Build Parity Ethereum from source, then start it with:


So it is not included in regular releases? That is a bit unusual so perhaps worth expanding on, like "The evmbin tool is not distributed with regular Parity Ethereum releases so you need to build it from source like so: …`?

i've pushed a commit fixing this, thanks!

Are you happy with the changes @dvdplm?

dvdplm · 2019-05-15T07:23:40Z

+//! Build Parity Ethereum from source, then start it with:
+//! ```bash
+//! cd evmbin && cargo build -p evmbin && cd ..
+//! ./target/debug/parity-evm --help


If built with --release the path becomes ./target/release/parity-evm (and we might want to suggest they copy the executable to, dunno, ~/bin?).

installing binaries is usually out of scope here

If they're using macOS or Linux and they want it in their PATH, we could suggest that they create a symlink i.e. sudo ln -s <ABSOLUTE_PATH>/parity-ethereum/target/release/parity-evm /usr/local/bin, assuming that echo $PATH indicates that /usr/local/bin is in their path, and then they can run parity-evm --help from any directory.
I'd have to look into what steps are required for Windows.

i've pushed a commit that informs the user the following: "If you wish to run parity-evm from any directory, then either copy the executable file itself to a directory that is in your PATH, or just create a symlink to it."

Is that ok @dvdplm @soc1c ?

Personally I'd avoid symlinks to the build directory because files therein are clobbered anytime I build. Maybe a user of evmbin doesn't build as often as I do, but who knows. I think the wording could just be: "if you wish to run parity-evm from any directory copy the executable from ./target/release to a directory in your $PATH, e.g. ~/bin or /usr/local/bin.".

Thanks, i've pushed a commit with this change

again, I would like to emphasize that explaining users how to use binaries is out of scope for our documentation.

@soc1c I've pushed another commit removing the explanation about how to use binaries

Co-Authored-By: David <dvdplm@gmail.com>

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

…e to PATH or create symlink

dvdplm

lgtm (other than nitpicks on some wording)

…of scope of docs

* master: Don't panic if extra_data is longer than VANITY_LENGTH (#10682) docs: evmbin - Update Rust docs (#10658) Remove annoying compiler warnings (#10679) Reset blockchain properly (#10669)

* master: add_sync_notifier in EthPubSubClient holds on to a Client for too long (#10689) Don't panic if extra_data is longer than VANITY_LENGTH (#10682) docs: evmbin - Update Rust docs (#10658) Remove annoying compiler warnings (#10679) Reset blockchain properly (#10669)

…lly typed serde serialization using Rust structs (#10657) * fix: Replace multirust with rustup wince multirust is deprecated * docs: Update evmbin Rust docs and code comments * WIP: Add Response struct. Initial step using serde to serialize instead of hardcoding with JSON * fix: Update Response struct types to be string after formatting * fix: Fix move out of borrowed content error by cloning informant * refactor: Change from camelcase to snake case to fix linting errors * restore: Restore some code since now covered in separate PR #10658 * restore: Restore original Rustdocs of evmbin * WIP * add Clone type * add newlines to end of json files * remove uml file that was unintentionally commited * rename chain spec to state test JSON fle * remove log. fix indentation * revert: Restore indentation now handled by separate PR #10740 * remove state test json files as moved to PR #10742 * revert changes in info.rs since covered in PR #10742 * revert changes to main.rs since covered in PR #10742 * revert newlines back to master * revert newlines back to master2 * refactor: Rename Response to TraceData * fix: Remove Clone and replace with lifetimes. Update tests since not ordered * refactor: Change all json! to typed serde * docs: Update rustdocs. Remove fixme * fix: Add missing semicolons from printf * fix: Change style from unwrap to expect in evmbin/src/display/json.rs Co-Authored-By: Andronik Ordian <write@reusable.software> * fix: Change style from unwrap to expect in evmbin/src/display/std_json.rs Co-Authored-By: Andronik Ordian <write@reusable.software> * revert updating module comments as will be done in separate PR #10742 instead * review-fix: Remove useless reference * Remove unncessary use of format macro * Update evmbin/src/display/json.rs Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/json.rs with serialization in set_gas success Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/json.rs with serialization in set_gas failure Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/std_json.rs with serialization in finish for state root Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/std_json.rs with serialization in before_test Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/std_json.rs with serialization for state root Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/std_json.rs with serialization for finish success Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Update evmbin/src/display/std_json.rs with serialization for finish failure Co-Authored-By: Andronik Ordian <write@reusable.software> * refactor: Rename structs and variables. Remove space. Simplify MessageInitial struct * refactor: Captialize expect message * revert to previous struct name TraceDataStateRoot * refactor: Simplify variable for consistency * Update accounts/ethstore/src/json/crypto.rs Co-Authored-By: David <dvdplm@gmail.com>

ltfschoen added 2 commits May 14, 2019 11:17

docs: Update evmbin to use rustup instead of multirust

9db14a4

docs: Update evmbin with Rustdocs overview, usage, comments

e2d4d4f

ltfschoen added the A0-pleasereview 🤓 Pull request needs code review. label May 14, 2019

soc1c added the M3-docs 📑 Documentation. label May 14, 2019

soc1c added this to the 2.6 milestone May 14, 2019

soc1c reviewed May 14, 2019

View reviewed changes

soc1c approved these changes May 14, 2019

View reviewed changes

jam10o-new approved these changes May 14, 2019

View reviewed changes

ltfschoen added a commit that referenced this pull request May 14, 2019

restore: Restore some code since now covered in separate PR #10658

a81a1b8

review-fix: Simplify evmbin usage commands

e851609

tomusdrw reviewed May 14, 2019

View reviewed changes

Comment thread evmbin/src/main.rs Outdated

dvdplm suggested changes May 15, 2019

View reviewed changes

ltfschoen and others added 8 commits May 15, 2019 11:11

docs: Update evmbin Overview typo

1d2cee6

Co-Authored-By: David <dvdplm@gmail.com>

docs: Update evmbin Overview grammer

17c4168

Co-Authored-By: David <dvdplm@gmail.com>

docs: Update evmbin Rust comments to be lowercase

57ad538

Co-Authored-By: David <dvdplm@gmail.com>

docs: Update evmbin Rust comments to be lowercase for gas price

eaca30a

Co-Authored-By: David <dvdplm@gmail.com>

docs: Update evmbin build command Rust docs

e928c33

Co-Authored-By: Tomasz Drwięga <tomusdrw@users.noreply.github.com>

review-fix: Update evmbin Overview to create release per dvdplm comment

d33a0ed

review-fix: Fix explanation in usage section

abc071f

review-fix: Update evmbin usage suggesting user to copy the executabl…

b6294e2

…e to PATH or create symlink

dvdplm approved these changes May 16, 2019

View reviewed changes

ltfschoen and others added 3 commits May 16, 2019 15:42

review-fix: Update evmbin executable in path sentence

27323a4

review-fix: Remove explaining to users how to use binaries since out …

46c597a

…of scope of docs

Merge branch 'master' into luke-evmbin-docs

3ad5cfd

sorpaas merged commit 8998774 into master May 21, 2019

sorpaas deleted the luke-evmbin-docs branch May 21, 2019 17:42

Conversation

ltfschoen commented May 14, 2019

Uh oh!

parity-cla-bot commented May 14, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ltfschoen commented May 14, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ltfschoen May 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

dvdplm left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ltfschoen May 15, 2019 •

edited

Loading