Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

docs: evmbin - Update Rust docs #10658

Merged
merged 14 commits into from
May 21, 2019
Merged

docs: evmbin - Update Rust docs #10658

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
9db14a4
docs: Update evmbin to use rustup instead of multirust
ltfschoen May 14, 2019
e2d4d4f
docs: Update evmbin with Rustdocs overview, usage, comments
ltfschoen May 14, 2019
e851609
review-fix: Simplify evmbin usage commands
ltfschoen May 14, 2019
1d2cee6
docs: Update evmbin Overview typo
ltfschoen May 15, 2019
17c4168
docs: Update evmbin Overview grammer
ltfschoen May 15, 2019
57ad538
docs: Update evmbin Rust comments to be lowercase
ltfschoen May 15, 2019
eaca30a
docs: Update evmbin Rust comments to be lowercase for gas price
ltfschoen May 15, 2019
e928c33
docs: Update evmbin build command Rust docs
ltfschoen May 15, 2019
d33a0ed
review-fix: Update evmbin Overview to create release per dvdplm comment
ltfschoen May 15, 2019
abc071f
review-fix: Fix explanation in usage section
ltfschoen May 15, 2019
b6294e2
review-fix: Update evmbin usage suggesting user to copy the executabl…
ltfschoen May 15, 2019
27323a4
review-fix: Update evmbin executable in path sentence
ltfschoen May 16, 2019
46c597a
review-fix: Remove explaining to users how to use binaries since out …
ltfschoen May 17, 2019
3ad5cfd
Merge branch 'master' into luke-evmbin-docs
sorpaas May 21, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion evmbin/benches/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
//! benchmarking for EVM
//! should be started with:
//! ```bash
//! multirust run nightly cargo bench
//! rustup run nightly cargo bench
//! ```

#![feature(test)]
Expand Down
20 changes: 19 additions & 1 deletion evmbin/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,21 @@
// You should have received a copy of the GNU General Public License
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.

//! Parity EVM interpreter binary.
//! Parity EVM Interpreter Binary.
//!
//! ## Overview
//!
//! The Parity EVM interpreter binary is an additional tool in the Parity
ltfschoen marked this conversation as resolved.
Show resolved Hide resolved
//! Ethereum toolchain. It is an EVM implementation for Parity Ethereum that
//! may be used to run a standalone version of the EVM interpreter.
ltfschoen marked this conversation as resolved.
Show resolved Hide resolved
//!
//! ## Usage
//!
//! Build Parity Ethereum from source, then start it with:
Copy link
Collaborator

Choose a reason for hiding this comment

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

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: …`?

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've pushed a commit fixing this, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you happy with the changes @dvdplm?

//! ```bash
//! cd evmbin && cargo build -p evmbin && cd ..
dvdplm marked this conversation as resolved.
Show resolved Hide resolved
//! ./target/debug/parity-evm --help
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

installing binaries is usually out of scope here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

@ltfschoen ltfschoen May 15, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

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.".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, i've pushed a commit with this change

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

//! ```

#![warn(missing_docs)]

Expand Down Expand Up @@ -268,13 +282,17 @@ struct Args {
}

impl Args {
/// Set the Gas Limit. Defaults to max value to allow code to run for whatever time is required.
ltfschoen marked this conversation as resolved.
Show resolved Hide resolved
pub fn gas(&self) -> Result<U256, String> {
match self.flag_gas {
Some(ref gas) => gas.parse().map_err(to_string),
None => Ok(U256::from(u64::max_value())),
}
}

/// Set the Gas Price. Defaults to zero to allow the code to run even if an account with no balance
ltfschoen marked this conversation as resolved.
Show resolved Hide resolved
/// is used, otherwise such accounts would not have sufficient funds to pay the transaction fee.
/// Defaulting to zero also makes testing easier since it is not necessary to specify a special configuration file.
pub fn gas_price(&self) -> Result<U256, String> {
match self.flag_gas_price {
Some(ref gas_price) => gas_price.parse().map_err(to_string),
Expand Down