Update check_icmp to use op5 version

[awiddersheim]

Looks like check_icmp has been enhanced by the op5 team but those enhancements haven't made it back into the plugins project.

http://git.op5.org/git/?p=nagios/op5plugins/check_icmp.git;a=blob;f=check_icmp.c;h=f559ea29c1991050519ab7093c3a7e1fe6606508;hb=HEAD

Just briefly looking at the op5 version it has much better help output and when trying it the intervals work as one would expect.

Perhaps it might be a good idea to pull in their version.

[catharsis]

If @weiss or someone else from the monitoring-plugins team could have a look at our fork and make a statement on what (if not everything) would be a good idea to upstream - I'd be happy to do the legwork of constructing the necessary PR's.

[weiss]

Ah sorry, I had only commented on IRC. Thanks a lot for your offer!

From what I saw, Andreas imported some version of check_icmp into the op5 repository early in 2007. Comparing that to our version from that time resulted in a >1000 line diff. Since then, 11 commits were applied to the op5 version, and something like 35 to ours, IIRC.

So, comparing our versions doesn't seem totally trivial. I was going to take a closer look, at least at those 11 commits, but didn't get to it yet. I'm happy about any concrete suggestions as to what we should pull, of course!

[catharsis]

I agree, it's definitely not trivial and, even though I'm not clear as to why, I think it's unfortunate that they've diverged.

How about you have a look at ours and see if there's something you definitiely do not want and vice versa? For an obvious example, I guess you don't care much about redirecting users to the op5-users mailing list for support.

Hopefully, we'll be able to reach some sort of consensus on what we want the final result (or at least a good approximation of it) to look like and I'll do my best to patch up the history from both repos. In the best of worlds, that'd allow us to kill off our fork entirely and just ship the upstream version to the benefit of everyone involved.

[weiss]

How about you have a look at ours and see if there's something you definitiely do not want and vice versa?

Sounds like a plan. Basically, I think we want everything that looks like it makes the world a better place :-)

We obviously don't want to loose previous fixes of ours; cd plugins-root; perl t/check_icmp.t shouldn't break; and for extra happiness, new stuff should come with new test cases where it makes sense.

I went through your commit messages (and some of the commit diffs) and they all look good to me (except for the obvious contact information changes you mentioned). I didn't look through the initial diff yet ...

[awiddersheim]

I mentioned to @weiss on IRC a few days ago when I initially created this issue that I was willing to help on making this better but @catharsis is probably much more qualified to do this than I am.

Either way the offer still stands if you want to throw some work my way.

[catharsis]

Sounds like I should join that channel. :-)

@awiddersheim Awesome! I'll give a shout if something comes up. Though I'm guessing it'll mostly be grunt work related to figuring out who did what when, and why and somehow putting the pieces together in a way that makes everyone happy.

[catharsis]

I took a look at this today. It seems there's a whole lot of history missing, unfortunately. And I'm unable to find anything close to a common point in history for the two repositories. I've asked Andreas about it, and he figures it might be the case that he had the missing history locally on a laptop at some point in time. In other words, I think we have to consider the chances of getting a clean history pretty slim.

I would suggest simply extracting and applying the desirable changes by hand. Though this of course requires a more thorough examination of what changes are desirable. I'm not familiar enough with the plugin to make any suggestions on this myself, but maybe @awiddersheim is (seeing as how he opened the issue with specific suggestions in the first place)?

Still, I'd be happy to help out in the "porting" part, or general discussion, if needed.

[awiddersheim]

@catharsis sorry for the late response. Been quite busy as of late. Thanks for starting to look at this. I originally had issues with the intervals using the monitoring-plugins version and the help that it gives isn't actually very helpful when it comes to explaining what the intervals do. If you download the two and run them with various i and I options and look at the help output I'm hoping you will see what I'm talking about.

That was the main factor that drove me to find that the op5 version which I found to be different/better in that area. Again, I haven't looked at the plugin or code very closely outside of those two things but I'd imagine there are probably other bugs/enhancements that would be worth fixing and having in the monitoring-plugins version.

[RincewindsHat]

It seems like this topic is quite dead. Well, op5 is also dead and the link is unreachable. If nobody here saved the the other version (and I have trouble finding it on first try) I would suggest to close the issue.

[jacobbaungard]

We (OP5) are still around. However it's our plan to start using the upstream monitoring-plugins version shortly. Perhaps there are still some things in our downstream version that could be useful, but it's probably not straight forward to upstream those changes, as per the previous discussions.

Anyway, I think it's fine we close this, I might have a look at some point later, and create PRs if I find anything useful in our downstream version. If anyone else want to have a go at it, let me know and I can provide the source.

[RincewindsHat]

Oh, I am sorry. It seems like I got the wrong impression from the first few links I found. Thanks for the feedback.