Patches for inclusion to Nagios 4

Ton Voon ton.voon at opsview.com
Thu Aug 8 15:17:01 CEST 2013


On 7 Aug 2013, at 08:51, Andreas Ericsson wrote:

> On 2013-08-06 19:16, Ton Voon wrote:
>> Hi!
>> 
>> We've published a list of patches for Nagios 4:
>> http://www.opsview.com/whats-new/blog/opsview-patches-nagios-4
>> 
>> We'd be happy if you could review if these are acceptable for future
>> inclusion or if anyone else finds them useful.
>> 
> 
> I'd like to get patches with commit messages and proper author and
> signed-off-by info. Since we're using git for Nagios now, it'd go
> a long way in making sure everyone gets credit for the work they've
> done.
> 
> The patches also need to apply cleanly to the latest master.
> 
> You may want to clone the official Nagios repo, apply your patches on
> top of it and then send me a pull request for github or some such.
> 
>   git clone git://git.code.sf.net/p/nagios/nagios nagios-core
> 
> should get you the very latest. If you apply your patches on top of
> 'master' and make sure to always do "git pull --rebase" when you want
> to get the latest and greatest you'll quickly see which patches either
> have been applied or which no longer *can* be applied. Then you can
> create a separate repository on github or some such and push the changes
> there.

OK, we'll convert them into git changes based off master. However....

I'd like an assurance that the changes will be merged before we promise to convert. Style changes are fine, won't take much time and will not require retesting, but if we need to refactor object changes or make larger changes to logic, this will require retesting on our side. I'd like a reassurance that the time invested will result in a merge upstream, otherwise we're just wasting time for all of us.

For instance, your assistance in the "environment macros per-command" was greatly appreciated and we've coded to the design agreed in the email conversations, but it hasn't been merged yet.

So I'd like to reverse the question and ask "which changes are most likely to be accepted, based on the amount of changes required" and we'll work through that list in order.

> 
> 
> On a first inspection though;
> * Don't comment out code. Bringing back dead code is what the VCS is
> for, and keeping it around is just plain dumb. If it shouldn't be in
> there, just remove it. In the same vein; Don't add commented-out code.
> 
> * Avoid C++ comments. I know they're supported in C99, which I'm rooting
> for as the least version supported, but it's against the current style.
> 
> * Don't mix spaces and tabs for indentation, unless it's continuation-
> indentation of a multi-line statement or condition. Stick to the style
> in the file you're editing, and *look* at the patch before you send it
> somewhere.
> 
> * Avoid comments saying things like "Opsview specific foobar" if you
> want to have the patches included. If you *don't* want those patches
> included, don't send them to me or point me to where they are. It takes
> up a lot of time to remove crap like that, and I have no interest in
> cleaning up after anyone else. I'm messy enough as it is on my own.
> 
> * Don't augment objects (such as hosts, services, commands) with new
> variables. Doing so means Nagios 4.1, and I can't take patches like
> that until Nagios 4 is at least released as stable. All objects have
> an 'id' field which means you can look up any extra data you want in
> O(1) time, provided you just initialize an array of size
> num_objects.$desired_object_type_in_plural before we enter the event
> loop.
> 
> * Make patches the most scalable you can. For the "check_time_period"
> thing, you'd be far better off adding code to detect timeperiod changes,
> notice which timeperiods are used to change commands and make a one-off
> swap for all the affected commands as the desired timeperiod comes
> either in or out of effect.
> 
> * Don't build on deprecated technology, such as external files for
> commands and/or check results

------------------------------------------------------------------------------
Get 100% visibility into Java/.NET code with AppDynamics Lite!
It's a free troubleshooting tool designed for production.
Get down to code-level detail for bottlenecks, with <2% overhead. 
Download for free and get started troubleshooting in minutes. 
http://pubads.g.doubleclick.net/gampad/clk?id=48897031&iu=/4140/ostg.clktrk




More information about the Users mailing list