Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 61.77%. Comparing base (5fc946d) to head (296f1ef).
⚠️ Report is 1 commits behind head on main.

@@            Coverage Diff            @@
##           main    #3510       +/-   ##
=========================================
+ Coverage      0   61.77%   +61.77%     
=========================================
  Files         0      644      +644     
  Lines         0    43775    +43775     
=========================================
+ Hits          0    27040    +27040     
- Misses        0    16735    +16735     

Please keep the following in mind:

• Proposal: ping: raise default wait time #3361 is a proposal, not a bug report; i.e., this here is not a "fix"
• this PR only covers changing the default value, does not add the proposed docs
• there is no conclusion on the proposal/discussion, not a single reply yet

Here is a suggestion:

Let's start with the documentation part. The numbers can be discussed.

I asked in Slack for people to take a look at the issue/proposal.

Thanks for the feedback.
I'll start with the documentation and update the PR, also change "Fixes" part.

I've added a description in the package comment in ping.go documenting how this implementation differs from iputils-ping.
I've also kept the default raised to 1000ms as proposed in the issue, but happy to revert that part if the Slack discussion concludes otherwise.

If you could split this up, i.e. put the description in another PR, I'd approve that one right away; the timeout change is smth I'd like feedback from others on, because I cannot judge it. :3

Sure, I'll open a separate PR with just the documentation change.

Let's make sure to squash when merging.
Thank you @rminnich for the feedback!

@nebojsaj1726 can you squash these commits to one?

oh yeah we do not have squash merging enabled, oops

@rminnich @orangecms I've cleaned up the branch to contain only the single relevant commit. Could you re-approve?

I got bitten by the lack of squash merge too, but it was explained to me that the resulting commit messages are very hard to follow.

I got bitten by the lack of squash merge too, but it was explained to me that the resulting commit messages are very hard to follow.

Yea that is a UI issue - you can adjust the message arbitrarily on GitHub, both summary and detailed description, and it's very much akin to a local git rebase, but it's hard to spot how and just clicking the button then gets a messy result, similar to when doing it locally.