[OPEN-ILS-DEV] PATCH: log.[ch] (localizing various things)

[Mike Rylander]

On 6/19/07, Scott McKellar <mck9 at swbell.net> wrote:
> --- Mike Rylander <mrylander at gmail.com> wrote:
>
> > I've attached a slight reworking of your patch for general
> > consumption.  Comments below.
> >
> > Thanks, Scott
> >
> > --miker
>
> Sorry but I'm a little confused now.
>
> Did you apply the modified patch?  The code in the Subversion
> repository doesn't show any changes, so far as I can tell.  Maybe
> I'm not looking in the right place?  What do I do now?

I apologize.  I had left it for the afternoon in case you wanted to
comment and then failed to commit in the evening.  It's in there now.

>
> I don't know what the customary practice is for a partially accepted
> patch.  I expected that you would apply the patch with whatever
> modifications you like.  Then I could download the fresh source
> files, as patched, and work from those.  However I can work with any
> reasonable procedure, as long as I know what it is.
>

Unless there's some non-obvious (for some definition thereof) reason
to change it we (obviously :P) just apply it without editorializing.
That's been the overwhelmingly common situation so far.

In cases such as this patch, which have been few, I prefer to kick
changes back out before unilaterally applying anything.  If you'd
prefer I skip that step for your submissions, unless there's something
very big (or it's a very big patch that I'd like wider comment on),
that's fine.  In any case I'll be more explicit in the future.  Sorry
for the confusion.

> Also: I can understand wanting to keep the macros for logging
> levels in the header.  But what about OSRF_LOG_GO?  That's not a
> logging level, it's a function-like macro that wraps the
> _osrfLogDetail function.  If the latter is moved out of the header
> and declared static, then OSRF_LOG_GO will not be usable outside
> of log.c.
>

OSRF_LOG_GO was moved into log.c as in your original patch, and
assuredly deserved to leave the header.

Thanks, Scott.

--miker

> Scott McKellar
> http://home.swbell.net/mck9/aargh/
>
> >
> > On 6/17/07, Scott McKellar <mck9 at swbell.net> wrote:
> > > This patch tidies up log.h and log.c a bit, mostly by moving
> > various
> > > things from the former to the latter.  In all cases I verified that
> > > no other source code referenced the things changed, apart from a
> > > few spurious matches in some python modules.
> > >
> > > 1. Whenever a declared identifier had two leading underscores, I
> > > removed the first one, except for a few cases where I removed the
> > > second one and kept the first one.
> > >
> > > 2. I moved the following macros to log.c:
> > >
> > >  OSRF_LOG_ERROR
> > >  OSRF_LOG_WARNING
> > >  OSRF_LOG_INFO
> > >  OSRF_LOG_DEBUG
> > >  OSRF_LOG_INTERNAL
> > >  OSRF_LOG_ACTIVITY
> > >  OSRF_LOG_GO
> >
> > For the time being I'm going to leave these defined in the header,
> > along with the osrfLogSetLevel() function, as I can certainly see the
> > need, during development of an application, to change the logging
> > level for a short section of code.  To support that, I'm going to add
> > a osrfLogGetLevel() function to retrieve the current level for later
> > resetting.
>
> <snip>
>
>


-- 
Mike Rylander