[lttng-dev] [REVIEW REQUEST babeltrace] Provide generic event payload accessors to the Python bindings

[jeremie.galarneau@efficios.com (Jérémie Galarneau)]

On Mon, Aug 26, 2013 at 11:48 AM, Mathieu Desnoyers
<mathieu.desnoyers at efficios.com> wrote:
> * J?r?mie Galarneau (jeremie.galarneau at efficios.com) wrote:
>> Hi all,
>>
>> I'd like to submit a review request for commits 34cf520 .. 3c5bb23 of
>> my bindings/python branch[1]. These are patches that were originally
>> contributed by Xiaona Han, that I have reviewed and to which I have
>> made minor changes (fixed typos and clarified some commit messages).
>
> Some comments:
>
> I notice e.g. in commit
> 34cf520df2f1df84a74a495a6f9a8d991f168c2e
> Return event fields by field name
>
> that the changelog appears to be really wide. In "git show", it goes
> beyond a 80-col screen. Please ensure that those changelogs are max 72
> lines in length, to make it easier to look at them in git show, and also
> to facilitate patch review, where they can easily become indented in
> reply messages,
>
> 6002606 Remove the unnecessary underscore prefix
> looks OK.
>
> fb0936d Support getting the value of enums
> looks OK.
>
>
> fb0ecd1c138a0b903090521406d239cf68b7e5e1
> Use a unique name to get a field's value
>
> +                       if id == ctf.type_id.ARRAY:
> +                               return self.get_char_array()
>
> I not sure if it is already checked somewhere, but if we have an array
> of e.g. uint64_t elements (not characters), is it OK to return a char
> array ?
>
> I'd be tempted to return a char array only if the encoding is ASCII or
> UTF8, and only if the array contains 8-bit characters. But maybe this is
> validated elsewhere ?
>
> 5891c4d Support for the sequence type
> looks OK
>
> 3c5bb23 Add a python bindings sequence test
> looks OK
>

Thanks for your comments. I have addressed them and rebased the
patches on the latest tree.
The new revisions are 91c9c98 .. f0a440b.

Regards,
J?r?mie

> Thanks,
>
> Mathieu
>
>
>
>
>>
>> Thanks,
>> J?r?mie
>>
>> [1] git://github.com/jgalar/babeltrace.git -b bindings/python
>> Web: https://github.com/jgalar/babeltrace/tree/bindings/python
>>
>> --
>> J?r?mie Galarneau
>> EfficiOS Inc.
>> http://www.efficios.com
>>
>> _______________________________________________
>> lttng-dev mailing list
>> lttng-dev at lists.lttng.org
>> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
J?r?mie Galarneau
EfficiOS Inc.
http://www.efficios.com