search

joelagnel / adeb

A debian-based shell environment designed for Android and adb
Apache License 2.0
319 stars 100 forks source link

Ssh remote #20

Closed qais-yousef closed 5 years ago

qais-yousef commented 5 years ago

This is still WIP in progress but I tested it on my busybox (buildroot) based target and it works fine for me. But probably needs more testing to iron more things out. It'd be good to get initial feedback at least and maybe get more people to try it with different envs.

I created a new utils/remote to abstract talking to the target. And had to do a few fixes to deal with limitations of my busybox env. Yes, my target isn't android based :-)

qais-yousef commented 5 years ago

This PR along with PR #19 address issue #18

joelagnel commented 5 years ago

I agree with the direction of the patches. The simple ones can also probably be split and can go in independently since those are nice to do. Thanks!

I hate how github doesn't number patches so one has no idea which one is the first, etc..
qais-yousef commented 5 years ago

Yes github confuses me too, especially when you get used to the nice views and facilities in gerrit..

I'll split things out and push another PR for them. Meantime if you can pull this branch and test it locally to make sure it doesn't break any of your workflow that'd be great! I tried on my pixel2 and didn't notice anything. Adb connection should work as it was after this change.

Thanks!

joelagnel commented 5 years ago

Sure, I'll give it a spin soon, thanks

On Thu, Feb 28, 2019 at 10:18 AM Qais Yousef notifications@github.com wrote:

Yes github confuses me too, especially when you get used to the nice views and facilities in gerrit..

I'll split things out and push another PR for them. Meantime if you can pull this branch and test it locally to make sure it doesn't break any of your workflow that'd be great! I tried on my pixel2 and didn't notice anything. Adb connection should work as it was after this change.

Thanks!

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/joelagnel/adeb/pull/20#issuecomment-468379987, or mute the thread https://github.com/notifications/unsubscribe-auth/AACSVHXeidF8a8uAYfqYuV4c9-LdmiH5ks5vSB2EgaJpZM4bWjD6 .

qais-yousef commented 5 years ago

Hi Joel. I am happier with this set of patches. It'd be nice for someone else other than me to try them out. I've used them with both android and my buildroot based systems without a problem. I fixed a few bugs in push/pull for the ssh remote.

Please review and consider merging.

Thanks!

qais-yousef commented 5 years ago

Hmm I just realized I might need to update the documentation too. I'll wait for your review first :-)

joelagnel commented 5 years ago

Other than the nits already mentioned, all patches LGTM. I am testing further now. Results also in a nice clean up so I love it, thank you!

qais-yousef commented 5 years ago

The FTP mirror commit was already merged, perhaps your PR needs to be updated with it?

A bad rebase or I forgot to update the master on my fork. I'll get it fixed in the next update. I'll write a few lines in README.md about ssh support too.

joelagnel commented 5 years ago

Ok, I tested these and the nits are minor, so I will merge this. Do consider providing README and taking care of the #! nit for future patches, thanks a lot!

qais-yousef commented 5 years ago

Great thanks!