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.

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

Rebased #12 - Updated to Rust 2018, ran Rustfmt, and fixed Clippy lints #14

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

usbalbin
Copy link

This is simply a rebased version of PR #12 thus all credit to the author of that PR.

Please note that I do not wish to step on anyone's toes :)

@usbalbin
Copy link
Author

In my local fork of this repo I have converted from 0.1 futures and tokio 0.1 to std futures with async await and tokio 0.2 . However this depends on this PR. Would there be any interest in me submitting a PR?

@usbalbin usbalbin requested a review from knsd January 27, 2020 11:27
@jcgruenhage
Copy link

Just came here to say that your fork works pretty well, first tried integrating this into a project, but after several incompatibility hurdles with the current ecosystem, I switched to your fork and it all became quite easy then. Thanks for your work!

@usbalbin usbalbin mentioned this pull request Jun 30, 2020
@endeav0r
Copy link

Is there any movement on this? Is this crate essentially dead?

@usbalbin
Copy link
Author

usbalbin commented Dec 18, 2020

I'm still waiting for a review

@jcgruenhage
Copy link

jcgruenhage commented Apr 19, 2021

@usbalbin ooi, have you by chance tried getting this to run on tokio 1? I've tried my best to port it to tokio 1 but while it does compile, after the first pong per chain it just times out forever. My changes are available on https://github.com/jcgruenhage/tokio-ping, in the wip commit on the master branch

With the help of the tokio-users discord channel, I've found what my problem was and now, it does actually work. I've started to do some more clean-up on tokio-ping (stuff bumping the remaining deps, fixing formatting, replacing failure with thiserror and so on), and unless @knsd wants to pick this up again, I'll probably go off and fork this.

@jcgruenhage
Copy link

So, no reply here, no reply to the email I wrote last week: Do we want to go ahead and fork this? Any suggestions for a new crate name for crates.io?

@usbalbin
Copy link
Author

Sorry for late response.

I can not speak for @knsd but to me a fork sounds like a plan :) What about just async_ping?

@jcgruenhage
Copy link

Sorry for late response.

No worries, I saw your 👍, I meant more "no response from upstream".

I can not speak for @knsd but to me a fork sounds like a plan :) What about just async_ping?

I dislike that a bit, because async_ping sounds runtime agnostic to me, which this is very much not. Considering we're (on a lower level) using ICMP echo requests for pinging, maybe tokio-icmp-echo? That's a bit obscure, so it hurds SEO, but with adding the keywords it should still be doable to pull over existing users.

@usbalbin
Copy link
Author

I dislike that a bit, because async_ping sounds runtime agnostic to me, which this is very much not.

That is fair.

Considering we're (on a lower level) using ICMP echo requests for pinging, maybe tokio-icmp-echo? That's a bit obscure, so it hurds SEO, but with adding the keywords it should still be doable to pull over existing users.

tokio-icmp-echo might not be the most flashy name, however it is accurate

@jcgruenhage
Copy link

jcgruenhage commented Apr 27, 2021

And done: https://crates.io/crates/tokio-icmp-echo. It's available under https://github.com/jcgruenhage/tokio-icmp-echo. @usbalbin I've invited you as a collaborator on the repo and on the crate on crates.io

@usbalbin
Copy link
Author

And done: https://crates.io/crates/tokio-icmp-echo. It's available under https://github.com/jcgruenhage/tokio-icmp-echo. @usbalbin I've invited you as a collaborator on the repo and on the crate on crates.io

I can not promise how much time I will be able to set aside for this but I would be happy to do what I can. :)

@jcgruenhage
Copy link

No worries there, I don't know that either, I just don't want to be alone on this so that the probability of no one answering MRs for years like it happened here is lower ^^

Having 2-3 people with write access means that even when one of them is hit by a bus, the others can keep going.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants