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.

Sign up for GitHub

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

Jump to bottom

Options "-c N" / "-C N" versus "-a" and "-u" (v2) #298

Merged
merged 3 commits into from
Feb 20, 2024
Merged

Options "-c N" / "-C N" versus "-a" and "-u" (v2) #298

merged 3 commits into from
Feb 20, 2024

Conversation

auerswal
Copy link
Collaborator

@auerswal auerswal commented Feb 7, 2024

  1. Adjust fping -h output to emphasize that -i MSEC does not change the output format.
  2. Ensure that neither -a nor -u affect -c N resp. -C N operation, and test this.
  3. Document in the man page that -c N and -C N override -a or -u.

This addresses issue #295. It should also help at least somewhat with issues #271 and #274.

@auerswal
Move description of -i MSEC to probing options
2097f34 
The -i MSEC, --interval=MSEC option does not directly affect
the output format, but rather the probe generation, i.e., the
minimum time between probes.  Thus it is more fitting to list
it under "probing options" than under "output options".
@auerswal
Decouple -a/-u effects from -c
0128877 
As described in issue schweikert#295, the option combinations "-c N -u"
and "-c N -a" inadvertently have the same effect as "-c N -q".
Prevent this by first acting on -c, which includes disabling
either -a or -u, if -c is given, before acting on -a/-u.
@auerswal
Document that -c N overrides -a or -u
818821f
@coveralls
Copy link

Coverage Status

coverage: 84.191% (-0.2%) from 84.423%
when pulling 818821f on auerswal:c_vs_a_and_u-v2
into d312a27 on schweikert:develop.

@auerswal
Copy link
Collaborator Author

auerswal commented Feb 7, 2024

Again I'd say that the reportedly reduced coverage is a bogus result. The pull request moves existing code around. All of the code was already tested (even though the observable output of combining -c N or -C N with either -a or -u was not), and as such the code coverage does not really change. Please consider merging anyway. Thanks in advance!

@auerswal
Copy link
Collaborator Author

auerswal commented Feb 7, 2024

I would like to adjust the fping -h output regarding both -c N and -C N a bit, without changing their positions:

Current:

   -c, --count=N      count mode: send N pings to each target
   -C, --vcount=N     same as -c, report results in verbose format

Suggestion:

   -c, --count=N      count mode: send N pings to each target and report stats
   -C, --vcount=N     same as -c, report results (not stats) in verbose format

Is this OK? If yes, I can add this as another commit to this pull request before you merge.

@schweikert
Copy link
Owner

I would like to adjust the fping -h output regarding both -c N and -C N a bit, without changing their positions:

Current:

   -c, --count=N      count mode: send N pings to each target
   -C, --vcount=N     same as -c, report results in verbose format

Suggestion:

   -c, --count=N      count mode: send N pings to each target and report stats
   -C, --vcount=N     same as -c, report results (not stats) in verbose format

Is this OK? If yes, I can add this as another commit to this pull request before you merge.

Sounds good to me. Thank you!

@schweikert schweikert merged commit ae0bc23 into schweikert:develop Feb 20, 2024
7 of 9 checks passed
@auerswal
Copy link
Collaborator Author

Thanks!

@auerswal auerswal deleted the c_vs_a_and_u-v2 branch February 21, 2024 19:15
@auerswal auerswal mentioned this pull request Feb 21, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@auerswal @coveralls @schweikert