search

openssh-rust / openssh

Scriptable SSH through OpenSSH in Rust
Apache License 2.0
232 stars 35 forks source link

support clean up the historical control_directory #126

Closed baoyachi closed 1 year ago

baoyachi commented 1 year ago

fix https://github.com/openssh-rust/openssh/issues/125

jonhoo commented 1 year ago

This change is Reviewable

codecov-commenter commented 1 year ago

Codecov Report

Merging #126 (24b9d35) into master (fefd900) will decrease coverage by 1.05%. The diff coverage is 24.00%.

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files | [Files Changed](https://app.codecov.io/gh/openssh-rust/openssh/pull/126?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [src/builder.rs](https://app.codecov.io/gh/openssh-rust/openssh/pull/126?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#diff-c3JjL2J1aWxkZXIucnM=) | `78.21% <24.00%> (-5.38%)` | :arrow_down: |
NobodyXu commented 1 year ago

P.S. I would recommend rebasing against main instead of merging main into this PR, which will create an additional commit.

jonhoo commented 1 year ago

I assume these mostly stuck around for when a connection failed, which has since been fixed (I think). I'm not in love with doing a recursive directory remove here invisibly on the user's behalf — feels like a recipe for accidentally deleting something unintentional down the line.

NobodyXu commented 1 year ago

I assume these mostly stuck around for when a connection failed, which has since been fixed (I think). I'm not in love with doing a recursive directory remove here invisibly on the user's behalf — feels like a recipe for accidentally deleting something unintentional down the line.

This behavior is disabled by default, so I think it might be fine.