fix: make conditions of ping more explicit #600
Conversation
I added the more conditions, because an unspecified timeout not only can mean that someone did not specify ms, but that, for example, did not specify a number in the timeout (and provided string, which we don't check) or specified a minus timeout (which is impossible?).
|
|
jennifer-shehane
changed the title
|
@MikeMcC399 Any thoughts on this. Seems fine. |
The PR is more than one year old, so it would need to be rebased. On first glance it looks to be good. The workflow example-wait-on.yml does not test for the error conditions that this PR attempts to cover, so these should be checked in a manual test. I have not seen any issues that this PR would solve, so if it is not merged, then I don't see any immediate impact. I suggest to see if @403-html is interested in reviving the PR and updating it. |
|
I am closing this stale PR. @403-html Please let us know if you want to work on this PR and update it. We can re-open it if necessary. |
|
Hi, thanks for notifying me @MikeMcC399, it was created after some folks struggled some time ago with not understandable timeout messages. It was quite bizarre case but not with any priority. Can stay closed, unless someone else would find it useful. |
|
It's good to hear from you and to know that you are still available if needed! Many thanks for replying! I can imagine that some of the timeout messages could be confusing. I also found that the debug output from ping is not so helpful and I started some work to improve that quite a long time ago. It didn't seem that users were having a problem anymore though so I didn't use that work. Let's leave this PR closed and as you say, if they is a need we can use it later. |
I added the more conditions, because an unspecified timeout not only can mean that someone did not specify ms, but that, for example, did not specify a number in the timeout (and provided string, which we don't check) or specified a minus timeout (which is impossible?).