[chirp_devel] [PATCH 1/7] Add chirp.logger module (#2347)

[Dan Smith]

> # HG changeset patch
> # User Zach Welch <zach at mandolincreekfarm.com>

Can you put the git hash into this block? When people send multiple
copies of the same patch before I get far enough to run the patchbot on
them, I use the NodeID to make sure I'm applying the thing I'm reviewing
in my mailer. The git hash would give me that for your patches as well.

> +#: Map human-readable logging levels to their internal values.
> +log_level_names = {"critical": logging.CRITICAL,
> +                   "error":    logging.ERROR,
> +                   "warn":     logging.WARNING,
> +                   "info":     logging.INFO,
> +                   "debug":    logging.DEBUG,
> +                   }

My point in the other patch was that you shouldn't replicate the level
names here when you can get them from the library. But I think the point
here is to let you do CHIRP_DEBUG=info, which means they needn't be the
canonical strings anyway, so, that's fine.

> +    def create_log_file(self, name):
> +        if self.logfile is None:
> +            self.logname = name
> +            lf = file(name, "w")
> +            print >>lf, version_string()
> +            lf.close()

What is the point of writing the raw string into the file before we open
it? That said, the above should be this for consistency:

 with file(name, 'w') as lf:
    print >>lf, version_string()

Which auto-closes the file. Since this is at the bottom of the stack,
I'll apply, but I think we should go back and just remove that (or turn
it into a log), unless there is some reason I'm missing.

--Dan

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
Url : http://intrepid.danplanet.com/pipermail/chirp_devel/attachments/20150226/4d025788/attachment-0001.bin