Great progress! A few more remarks: * Class methods ought to be verbs. "this_machine" and "this_boot" are the key ones that come to mind. Maybe "add_machine_match" and "add_boot_match"? * In methods like "this_machine" that have special handling for the None value as "myself," the function should use something more meaningful. You could even define a constant like CURRENT_MACHINE as None. It's just unclear in calling code that myjournal.this_machine(None) matches on the current machine versus, say, no machine field, an empty machine field, or *any* value in the machine field. * "get_next" should note in its documentation that, combined with the native C, one can use a Pythonic iterator directly. I would even make this function private to encourage use of the iterator syntax. * The "except" handler for _convert_unicode() should be more specific. I think it's generally dangerous to have catch-all exceptions, especially ones that fail silently. I couldn't find in the Python docs for 2 or 3 what the exception would be.
-- David Strauss | da...@davidstrauss.net | +1 512 577 5827 [mobile] On Fri, Feb 15, 2013 at 9:28 AM, Steven Hiscocks <steven-syst...@hiscocks.me.uk> wrote: > On 11/02/13 05:49, Zbigniew Jędrzejewski-Szmek wrote: >> >> On Fri, Feb 08, 2013 at 10:20:57PM +0000, Steven Hiscocks wrote: >>> >>> I thought the easiest way was to just inherit the Journal (now >>> _Journal), replacing __new__ with all the python RunString stuff. >>> https://github.com/kwirk/systemd/commit/9003fdbc69577840ab6a1501a1c49f7377b6124d >> >> >> Hi Steven, >> comments on your patch: >> >> The default conversion must be bytes, otherwise every new binary >> message field that this module doesn't know about will cause failure. >> It is better to default to bytes. >> > Currently I've put default to unicode, which falls back to bytes if their is > an exception is raised. >> >> ... >> >> Zbyszek >> > > I've been away on a work trip this week so haven't been able to make much > progress. I've pushed some changes to a branch on github: > https://github.com/kwirk/systemd/tree/python-systemd-reader > > -- > Steven Hiscocks -- David Strauss | da...@davidstrauss.net | +1 512 577 5827 [mobile] _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel