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

whisper/cli: add p2p port and ip parameters #10057

Merged
merged 11 commits into from
Mar 21, 2019
Merged

whisper/cli: add p2p port and ip parameters #10057

merged 11 commits into from
Mar 21, 2019

Conversation

gballet
Copy link
Contributor

@gballet gballet commented Dec 12, 2018

This is so that those params don't change randomly and are in sync with the URL that
is displayed.

@parity-cla-bot
Copy link

It looks like @gballet hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@Tbaut Tbaut added A0-pleasereview 🤓 Pull request needs code review. M2-config 📂 Chain specifications and node configurations. labels Dec 12, 2018
@Tbaut Tbaut added this to the 2.3 milestone Dec 12, 2018
@niklasad1 niklasad1 self-requested a review December 12, 2018 17:25
niklasad1
niklasad1 reviewed Dec 12, 2018
View reviewed changes
whisper/cli/src/main.rs Outdated Show resolved Hide resolved
niklasad1
niklasad1 reviewed Dec 12, 2018
View reviewed changes
whisper/cli/src/main.rs Outdated Show resolved Hide resolved
niklasad1
niklasad1 reviewed Dec 12, 2018
View reviewed changes
whisper/cli/src/main.rs Outdated Show resolved Hide resolved
niklasad1
niklasad1 suggested changes Dec 12, 2018
View reviewed changes
Copy link
Collaborator

@niklasad1 niklasad1 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Just fix the panic that may happen on invalid IP Address which causes one test to fail. Otherwise good to go 👍

@niklasad1 niklasad1 added A8-looksgood 🦄 Pull request is reviewed well. A4-clasignoffneeded 📛 Pull request requires author to sign off on CLA before review/merge can begin. and removed A0-pleasereview 🤓 Pull request needs code review. labels Dec 17, 2018
@niklasad1
Copy link
Collaborator

Optional, rebase to master to fix cargo audit however it is fine to leave it because we squash-merge in the end!

@5chdn 5chdn removed the A8-looksgood 🦄 Pull request is reviewed well. label Jan 2, 2019
5chdn
5chdn suggested changes Jan 2, 2019
View reviewed changes
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.

could you consider a minor version bump for the crate?

also, could you sign the CLA?

@5chdn 5chdn modified the milestones: 2.3, 2.4 Jan 10, 2019
@5chdn 5chdn added the A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Jan 10, 2019
@5chdn
Copy link
Contributor

5chdn commented Jan 10, 2019

Please, reopen the PR once you signed the CLA.

@5chdn 5chdn closed this Jan 10, 2019
@niklasad1 niklasad1 mentioned this pull request Jan 15, 2019
Closed
@gballet
Copy link
Contributor Author

gballet commented Mar 15, 2019

@5chdn @Tbaut @niklasad1 I finally got the authorization to sign the CLA 🎉 . Would you mind re-opening the PR, I don't seem to be able to.

@ordian ordian reopened this Mar 15, 2019
@soc1c
Copy link
Contributor

soc1c commented Mar 15, 2019

[clabot:check]

@parity-cla-bot
Copy link

It looks like @gballet hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@gballet gballet force-pushed the whisper-cli-specify-p2p branch from 317c69f to 3754ddd Compare March 18, 2019 13:06
@gballet
Copy link
Contributor Author

gballet commented Mar 19, 2019

@niklasad1 could you have another look please?

@soc1c
Copy link
Contributor

soc1c commented Mar 19, 2019

Can you please sign the CLA?

@soc1c soc1c removed the A3-stale 🍃 Pull request did not receive any updates in a long time. No review needed at this stage. Close it. label Mar 19, 2019
@gballet
Copy link
Contributor Author

gballet commented Mar 19, 2019

I just tried to sign it again, it says it's already done

@soc1c
Copy link
Contributor

soc1c commented Mar 19, 2019

Investigating.

@gballet
Copy link
Contributor Author

gballet commented Mar 19, 2019

thanks!

@ddorgan
Copy link
Collaborator

ddorgan commented Mar 19, 2019

[clabot:check]

@parity-cla-bot
Copy link

It looks like @gballet signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

niklasad1
niklasad1 reviewed Mar 19, 2019
View reviewed changes
whisper/cli/src/main.rs Outdated Show resolved Hide resolved
@ordian
Copy link
Collaborator

ordian commented Mar 19, 2019

Could you update whisper/README.md as well? Looks good otherwise.

ordian
ordian reviewed Mar 19, 2019
View reviewed changes
whisper/cli/Cargo.toml Outdated Show resolved Hide resolved
@soc1c soc1c added A8-looksgood 🦄 Pull request is reviewed well. and removed A4-clasignoffneeded 📛 Pull request requires author to sign off on CLA before review/merge can begin. labels Mar 19, 2019
@soc1c soc1c modified the milestones: 2.4, 2.5 Mar 19, 2019
ordian
ordian approved these changes Mar 20, 2019
View reviewed changes
Cargo.lock Outdated Show resolved Hide resolved
whisper/README.md Outdated Show resolved Hide resolved
@gballet
Copy link
Contributor Author

gballet commented Mar 21, 2019

Is it possible to restart the failing builds? They failed because the build took too long.

@niklasad1
Copy link
Collaborator

@gballet Yes, I can do it but I think the problem is the CI setup is too old.

Can you merge/rebase to master instead?

gballet and others added 11 commits March 21, 2019 15:16
@gballet
whisper/cli: add p2p port and ip parameters
bf4b3eb 
This is so that those params don't change randomly and are in sync with the URL that
is displayed.
@niklasad1 @gballet
feedback: Result instead of panic
ab803e7 
Co-Authored-By: gballet <[email protected]>
@niklasad1 @gballet
feedback: Map error in port conversion
95b1d9a 
Co-Authored-By: gballet <[email protected]>
@gballet
whisper/cli: User can specify enode private key
5480b76 
So that the enode doesn't change at every run.
@gballet
whipser/cli: finish integrating review feedback.
72cbf2e
@gballet
Accomodate error API change
2afee32
@ordian @gballet
Update rustc-hex version in whisper/cli/Cargo.toml
5cedb7b 
Co-Authored-By: gballet <[email protected]>
@gballet
Update README with new whisper cli options
eb3e6ef
@niklasad1 @gballet
Fix typo in error message
d802432 
Co-Authored-By: gballet <[email protected]>
@gballet
Fix Cargo.lock and build issue after lib version upgrade
27e0fc9
@ordian @gballet
Fix another typo
2cf76ed 
Co-Authored-By: gballet <[email protected]>
@gballet gballet force-pushed the whisper-cli-specify-p2p branch from 154313b to 2cf76ed Compare March 21, 2019 14:16
@gballet
Copy link
Contributor Author

gballet commented Mar 21, 2019

Done

@ordian ordian merged commit b700ff3 into openethereum:master Mar 21, 2019
ordian added a commit that referenced this pull request Apr 5, 2019
@ordian
Merge branch 'master' into ao-lmdb
075b8d4 
* master: (48 commits)
  ethcore: remove eth social and easthub chain configs (#10531)
  Initial support sccache for windows build (#10520)
  fix(light): make `OnDemand` generic instead of using the concrete type (#10514)
  private-tx: replace error_chain (#10510)
  Add trace information to eth_estimateGas (#10519)
  ethcore: add clique engine (#9981)
  verbose flag for cpp tests (#10524)
  Add a more realistic Batch test (#10511)
  docs: add changelogs for 2.3.{6,7,8} and 2.4.{1,2,3} (#10494)
  fix(rpc): fix a bunch of clippy lints (#10493)
  fix Sha3/keccak256 hash calculation for binaries (#10509)
  Add additional request tests (#10503)
  whisper/cli: add p2p port and ip parameters (#10057)
  fix(time-utils): add missing license (#10497)
  fix(extract `timestamp_checked_add` as lib) (#10383)
  fix(rpc): lint `unused_extern_crates` + fix warns (#10489)
  fix win&mac build (#10486)
  Сaching through docker volume (#10477)
  OpenBlock::new take IntoIterator instead of mutable ref to Iterator (#10480)
  simplify block module and usage (#10479)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@niklasad1 niklasad1 niklasad1 approved these changes

@5chdn 5chdn 5chdn requested changes

@ordian ordian ordian approved these changes

Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well. M2-config 📂 Chain specifications and node configurations.
Projects
None yet
Milestone
2.5
Development

Successfully merging this pull request may close these issues.
8 participants
@gballet @parity-cla-bot @niklasad1 @5chdn @soc1c @ddorgan @ordian @Tbaut