Re: [Nagios-devel] Patches for inclusion to Nagios 4

Support forum for Nagios Core, Nagios Plugins, NCPA, NRPE, NSCA, NDOUtils and more. Engage with the community of users including those using the open source solutions.
Locked
Guest

Re: [Nagios-devel] Patches for inclusion to Nagios 4

Post by Guest »


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

> On 2013-08-06 19:16, Ton Voon wrote:
>> Hi!
>>=20
>> We've published a list of patches for Nagios 4:
>> http://www.opsview.com/whats-new/blog/o ... s-nagios-4
>>=20
>> We'd be happy if you could review if these are acceptable for future
>> inclusion or if anyone else finds them useful.
>>=20
>=20
> 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.
>=20
> The patches also need to apply cleanly to the latest master.
>=20
> 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.
>=20
> git clone git://git.code.sf.net/p/nagios/nagios nagios-core
>=20
> 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.

>=20
>=20
> 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.
>=20
> * 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.
>=20
> * 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.
>=20
> * 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.
>=20
> * 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.
>=20
> * 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.
>=20
> * Don't build on deprecated technology, such as external files for
> commands and/or check results





This post was automatically imported from historical nagios-devel mailing list archives
Original poster: ton.voon@opsview.com
Locked