@sni / @waja - Could we please have some opportunity for review / discussion here before immediately squashing this as closed? I would be more than happy to provide additional / alternate revisions here, if/as required for inclusion. I think we'd all hate to have an additional fork here for something so simple.
> since you almost always have the hostname in your interface or > notification already and we'd like to avoid duplicate information. This is backwards of what I intended. As I noted in the original pull request above, I am monitoring everything by its DNS name, not IP. This is for the reasons above, and is what I believe to be a better monitoring configuration. As such, yes, we'd already have the hostname - but the IP is then left missing. This is why "ping" provides these details for this very reason - pinging "myHost.com"? Great - but which host did we resolve to? If the IP changed to something "invalid" which is what caused the failure, this option would allow us to see that immediately - without any additional digging required, but only if enabled with the run-time option. Again, we're not adding anything "extra" here - just no longer always preventing useful information that the "ping" output already provided us with from making it to the output. > it still makes the code more complicated and less maintable 16 additional lines of code, largely templated off of the existing code? > and reduces the number of available options for useful future features Are we talking available command-line flags here? Let's just assume 36 single-characters to begin with ([A-Za-z0-9]), not including '?' and other non-disputed allowed flags. We're currently using 12, and this would make 13. If we'd like to avoid using a single-character flag for this, that's fine - and we can just stick with the long "--show-resolution" option. > Also the whitespace seemed ok already, according to our guidelines (see > CODE file) there should be leading tabs followed by spaces. I completely agree. However, if you review the current version of the file, the indentation is not consistent. In re-reviewing my changes, the first 3 whitespace-related edits I made are probably up for debate, and I should have left them alone. However, throughout the rest of the file - indentations were being made with spaces only - not the "leading tabs followed by spaces" which are required by the CODE file, and a preference that I agree with and support. > that should be done seperatly. This is exactly why I made the whitespace changes in a separate commit - but given that I was in this file anyway, I was hoping that a separate branch and pull request would not be required here. What can we please do to compromise here? If I submit a new pull request with only the "--show-resolution" flag and supporting changes - no additional single-character flag and no whitespace changes, will you please reconsider? (Or is there anything we can please work with here without even requiring all of that?) Thanks! -- Reply to this email on GitHub: https://github.com/monitoring-plugins/monitoring-plugins/pull/1284#issuecomment-64939909
