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.

Sign up for GitHub

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

Jump to bottom

Add --all option to light rm command to remove all peers #432

Merged
merged 4 commits into from
Dec 8, 2020
Merged

Add --all option to light rm command to remove all peers #432

merged 4 commits into from
Dec 8, 2020

Conversation

romac
Copy link
Member

@romac romac commented Dec 1, 2020

Closes: #431

Description

Add --all option to light rm command to remove all peers for a given chain.

For contributor use:

  • Updated the Unreleased section of CHANGELOG.md with the issue.
  • If applicable: Unit tests written, added test to CI.
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
  • Updated relevant documentation (docs/) and code comments.
  • Re-reviewed Files changed in the Github PR explorer.

@adizere adizere modified the milestone: v0.0.6 Dec 3, 2020
ancazamfir
ancazamfir reviewed Dec 3, 2020
View reviewed changes
relayer-cli/src/commands/light/rm.rs
Comment on lines +38 to +39
/// remove all peers, implies --force
all: bool,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we have a flag to skip confirmation?

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, I forgot about that, will get on it!

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@codecov-io
Copy link

codecov-io commented Dec 3, 2020 •
edited
Loading

Codecov Report

Merging #432 (878550e) into master (b1b37f5) will increase coverage by 22.9%.
The diff coverage is 68.3%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #432      +/-   ##
=========================================
+ Coverage    13.6%   36.6%   +22.9%     
=========================================
  Files          69     154      +85     
  Lines        3752   11114    +7362     
  Branches     1374    4268    +2894     
=========================================
+ Hits          513    4072    +3559     
- Misses       2618    6429    +3811     
+ Partials      621     613       -8
Impacted Files Coverage Δ
modules/src/events.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/events.rs 0.0% <ø> (ø)
modules/src/ics02_client/msgs.rs 0.0% <0.0%> (ø)
modules/src/ics02_client/raw.rs 0.0% <0.0%> (ø)
modules/src/ics03_connection/error.rs 13.6% <0.0%> (-19.7%) ⬇️
modules/src/ics04_channel/error.rs 75.0% <ø> (+50.0%) ⬆️
modules/src/ics04_channel/msgs/chan_close_init.rs 56.8% <ø> (ø)
modules/src/ics04_channel/msgs/chan_open_ack.rs 64.2% <ø> (ø)
...odules/src/ics04_channel/msgs/chan_open_confirm.rs 61.9% <ø> (ø)
modules/src/ics04_channel/msgs/chan_open_init.rs 67.0% <ø> (ø)
... and 286 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update be001eb...878550e. Read the comment docs.

adizere
adizere approved these changes Dec 8, 2020
View reviewed changes
Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

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

Is this ready? Looks good to me!

ancazamfir
ancazamfir approved these changes Dec 8, 2020
View reviewed changes
Copy link
Collaborator

@ancazamfir ancazamfir left a comment

Choose a reason for hiding this comment

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

Thanks Romain!

@romac romac merged commit 1bd058d into master Dec 8, 2020
@romac romac deleted the romac/rm-all branch December 8, 2020 15:45
hu55a1n1 pushed a commit to hu55a1n1/hermes that referenced this pull request Sep 13, 2022
@romac
Add --all option to light rm command to remove all peers (informals…
9377a3e 
…ystems#432)

* Add --all option to `light rm` command to remove all peers

* Add --yes option to skip confirmation

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adizere adizere adizere approved these changes

@ancazamfir ancazamfir ancazamfir approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add option to light rm command to remove all peers
4 participants
@romac @codecov-io @adizere @ancazamfir