On Mon, Aug 26, 2013 at 11:48 AM, Mathieu Desnoyers <[email protected]> wrote: > * Jérémie Galarneau ([email protected]) 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 >> [email protected] >> 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 _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
