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

Phrasing, precisions and typos in CLI help #9060

Merged
merged 3 commits into from
Jul 10, 2018

Conversation

Tbaut
Copy link
Contributor

@Tbaut Tbaut commented Jul 6, 2018

No description provided.

@Tbaut Tbaut added A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M3-docs 📑 Documentation. labels Jul 6, 2018
@parity-cla-bot
Copy link

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

Many thanks,

Parity Technologies CLA Bot

@Tbaut Tbaut unassigned 5chdn Jul 6, 2018
@Tbaut Tbaut requested a review from 5chdn July 6, 2018 13:47
@Tbaut Tbaut changed the title Phrasing and typos in CLI help Phrasing, precisions and typos in CLI help Jul 6, 2018
@5chdn 5chdn added this to the 2.0 milestone Jul 6, 2018
@5chdn
Copy link
Contributor

5chdn commented Jul 6, 2018

Thanks for this, it will be a mess in combination with #9036 though :D

@Tbaut
Copy link
Contributor Author

Tbaut commented Jul 6, 2018

we can wait and amend/merge it later

@Tbaut Tbaut added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 6, 2018
Rephrase cli subcommand descriptions.
@5chdn 5chdn added A0-pleasereview 🤓 Pull request needs code review. and removed A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. labels Jul 9, 2018
@Tbaut
Copy link
Contributor Author

Tbaut commented Jul 10, 2018

@5chdn looks like there is no conflict :D

Copy link
Collaborator

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

lgtm with minor grumbles

@@ -82,7 +82,7 @@ usage! {

CMD cmd_import
{
"Import blockchain",
"Import blockchain from a file to the given --chain database (default: foundation)",
Copy link
Collaborator

Choose a reason for hiding this comment

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

dumb q: are we talking abouta blockchain spec here? With this wording it sounds a little like we're importing data into a db.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's indeed the data. Added it.

@@ -99,7 +99,7 @@ usage! {

CMD cmd_export_blocks
{
"Export blocks",
"Export blocks into the given --chain database (default: foundation). This command requires the chain to be synced with --fat-db on.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should it be Export blocks **from** the given --chain database?

Also "This command requires that the chain is synced with --fat-db"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • yup for the "from"
  • I think

This command requires the chain to be synced with --fat-db on.

is correct

Copy link
Collaborator

Choose a reason for hiding this comment

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

works for me

@@ -198,7 +198,7 @@ usage! {

CMD cmd_restore
{
"Restore database from snapshot",
"Restore database of the given --chain (default: foundation) from snapshot",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"Restore the database … …"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@@ -211,7 +211,7 @@ usage! {

CMD cmd_tools_hash
{
"Hash a file",
"Hash a file using Keccak-256 algorithm",
Copy link
Collaborator

Choose a reason for hiding this comment

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

"…using the Keccak…"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@5chdn 5chdn left a comment

Choose a reason for hiding this comment

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

Just a few comments, but lgtm.

}

CMD cmd_account_import
{
"Import account",
"Import accounts from JSON UTC keystore files to the specified --chain (default mainnet)",
Copy link
Contributor

Choose a reason for hiding this comment

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

mainnet? foundation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mainnet everywhere now.

@@ -157,11 +157,11 @@ usage! {
"Manage signer",

CMD cmd_signer_new_token {
"Generate new token",
"Generate new token for the given --chain (default: foundation)",
Copy link
Contributor

Choose a reason for hiding this comment

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

Generate a new signer-authentication token...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}

CMD cmd_signer_list {
"List",
"List the tokens from given --chain (default: foundation)",
Copy link
Contributor

Choose a reason for hiding this comment

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

signer-authentication tokens?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@5chdn 5chdn added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Jul 10, 2018
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels Jul 10, 2018
@5chdn 5chdn merged commit cd58b5f into master Jul 10, 2018
@5chdn 5chdn deleted the Tbaut-patch-phrasing-and-typos branch July 10, 2018 10:17
dvdplm added a commit that referenced this pull request Jul 10, 2018
* master:
  Delete crates from parity-ethereum and fetch them from parity-common instead (#9083)
  Updater verification (#8787)
  Phrasing, precisions and typos in CLI help (#9060)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). A8-looksgood 🦄 Pull request is reviewed well. M3-docs 📑 Documentation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants