Closed masklinn closed 1 year ago
@NobodyXu can you approve workflow runs? Also maybe comment on this early starting point.
Merging #136 (bbe309a) into master (499c672) will increase coverage by
0.00%
. The diff coverage is83.33%
.
:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.
P.S. I don't think implementing Command<Session>
is a good idea, since it's never designed to be taken by value.
Command<Arc<Session>>
or Command<Rc<Session>>
is more similar to Command<&Session>
and I'd like to see support for them.
P.S. I don't think implementing
Command<Session>
is a good idea, since it's never designed to be taken by value.
I was mostly thinking of the case where a user might want to run a single command on the session, but didn't want to bother with borrowing. However as I wrote up it doesn't work without changing the Command
API entirely so that's moot.
[raw_]command
go through to_[raw_]command
since they now canCommand
to OwningCommand
instead of just aliasing it as the crate root, for documentary clarityto_subcommand
associated function to do the session-owning thing for subcommand featuresBTW, you need to fix the CI before I can merge this PR.
BTW, you need to fix the CI before I can merge this PR.
Yep but since I can't run the tests locally I can only push and wait for CI approval. I'll get on that.
@masklinn There's a run_ci_tests.sh
in the repository that can be used to run some of the tests.
For linting, it's just cargo-clippy
and cargo-fmt
.
There's a run_ci_tests.sh in the repository that can be used to run some of the tests.
Yeah but it requires docker, and I'm on a mac and not super willing to install docker desktop. It's nbd as far as I'm concerned just a bit more latency but I'm not replying within seconds anyway. Unless you have to approve every CI run I can imagine that gets old.
For linting, it's just
cargo-clippy
andcargo-fmt
.
Yeah I ran fmt but it changed imports I had not touched so I figured the repo had strange configuration and reverted them, and they show up as lint errors so I guess it's just that they were last fmt'd before a change in configuration or something.
Hopefully CI is good now, I broke one of the existing doc comments by mistake, and the new arc_command
example had a few brainfarts.
Yeah but it requires docker, and I'm on a mac and not super willing to install docker desktop.
You can use OrbStack, an alternative to docker desktop that claims to be faster than docker desktop and uses less energy.
I use it for docker desktop now and it indeed startup quickly.
It's nbd as far as I'm concerned just a bit more latency but I'm not replying within seconds anyway. Unless you have to approve every CI run I can imagine that gets old.
Yes, I do need to approve every CI run.
Once this PR lands in main, any new PR from you will have CI run automatically without having me to approve.
so I guess it's just that they were last fmt'd before a change in configuration or something.
It's probably because new lints are added to newer cargo.
The coverage failure is probably due to CI setup (e.g. GHA does not allow PRs from non-member to use secrets) or whatever, I can bypass that and merge the PR once you address my review.
Thank you @masklinn !
Released in v0.10.1
Pushing this to run the tests as getting things to work locally on a mac seems like a bit of a bear.
RemoteChild::session
(the lifetime becomes bound to the child's rather than the original session's), so move it into its own impl block, and added an extremely debatableChild::into_session
, since it means not being able to callwait
, orwait_with_output
, ordisconnect
, so probably not useful (but a slightly separate variant ofChild::session
seems awful).Session
, but that turns out impossible as theCommand
builder works off of mutable references, so not possible to consume the builder in order to get the session out, short of creating either separateimpl
blocks forCommand<&Session>
,Command<Arc<Session>>
andCommand<Session>
, or separate consuming spawns, neither sounding great.Naming is dubious, especially
shared_command
, but I don't recall ever seeing a method around shared refs so I went simple. I would have impl'dArc<Session>::into_command
but that's not legal...Maybe a free function would be better?
Hopefully will eventually fix #117