On 2013-08-08 15:17, Ton Voon wrote:
>
> 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/o ... s-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.
>
It has, but it hasn't. The original patch no longer applies, due to
extensive remodeling of the worker code. I still have it hanging
around though, and I'm fairly inclined towards using it for services
and hosts as well, so that environment macros can be set on a service
level as well as on a command level. I've also been investigating the
chances of doing this without resorting to setenv() (in essence,
building a one-off block of the load-time environment macros, and then
extending that whenever we hit an environment variable).
But yea, I've got that one already.
> 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.
>
Thread-safe calls
It's harmless, and I'd apply it straight away if I had a sensible
commit message for it, but I don't know why you need this so I can't
really write one. While we're on the topic; Please write commit
messages in imperative form and present tense, as if you're giving
orders to the code for how it should change. Also give a short
rationale for why it should change that way. The rules about not
commenting out code you want to get rid of still applies though. It
just looks terribly hackish when patches meant for upstream contains
things like that.
Slice services within hosts
So long as it's configurable from cgi.cfg and the default stays the
same as it is today I'll apply it immediately. It has no impact on
loadable modules or other headaches, so that's a nobrainer, really.
Check command by time period
I feel this is somewhat lacking in efficiency and flexibility, and
a much cleaner solution would be to add a filtering functionality to
NERD so that checkresults can be shipped off to a third-party addon
which can transform checkresults and plugin output as much as it
likes.
Failing that (which would be enormously cool but also a lot of work),
I think it would most likely be best off as a separate module, with
custom variables or separate configuration to support it. Supporting
patches to run events when a timeperiod becomes a
...[email truncated]...
This post was automatically imported from historical nagios-devel mailing list archives
Original poster: ae@op5.se