Hi Steven, On Thu, 2009-04-16 at 16:28 -0700, Steven Dake wrote: > Great work! > comments that should be addressed: > > logsys.h: > > > please no single line comments, especially in header files. can you > multiline this or put it in the comment above the definitions?
Absolutely not a problem. I'll fix them all (also the one you spotted below). > > /* FORK and THREADED are ignored for SUBSYSTEMS */ > > How would you feel about changing LOG_MODE_* to LOGSYS_MODE_*? I'd > prefer all defines in public headers use LOGSYS as starters. It can be done. It just needs another report of all of corosync but not a big deal. > /* logsys_logger bits */ > #define SUBSYS_MAX 63 /* maximum number of subsystems */ > #define SUBSYS_MAXLEN 64 /* max len of a subsystem name */ > ^^single line comments are evil imo please fix > If this is going to be in a public header, it should be prefixed with > LOGSYS_ (such as LOGSYS_SUBSYS_MAX). I changed them to: /* logsys_logger bits */ #define MAX_SUBSYS_COUNT 64 /* maximum number of subsystems */ #define MAX_SUBSYS_NAMELEN 64 /* max len of a subsystem name */ in my tree (patch not posted yet) on Jim suggestion. I can change them to: LOGSYS_MAX_ if that's ok with you. > > can you change LOG_REC_END to LOGSYS_REC_END? sure. > > tagnames has improper tabbing (line 75). Ok. > > Really great work. If you like, please add your name to the license > section of the header and c file. Thanks! I'll finish today the port of corosync (only need the mainconfig.c rework testing) and publish the whole thing for review. Fabio _______________________________________________ Openais mailing list [email protected] https://lists.linux-foundation.org/mailman/listinfo/openais
