Troy Curtis Jr wrote on Sun, 16 Dec 2018 09:59 -0500:
> But there was one item I wanted to talk about related to the patch. I
> agree that "svn_stringbuf_t" => "bytes" and "svn_string_t" => "str" is the
> right general path,
Are you sure about this? Property values can be binary data (so not
representable as 'str') but are usually represented by «svn_string_t *»;
for example, svn_repos_parse_fns3_t::set_node_property().
> but after the patch there is now a "svn_stringbuf_t
> *output" typemap that is still using "PyStr" instead of "PyBytes".
> Further, the only usage of "svn_stringbuf_t *output" is in
> "svn_fs_print_modules" and "svn_ra_print_modules", where the function
> parameter is actually an output parameter, so the appropriate typemap is
> probably "argout" instead of "in".
>
> Otherwise, the patch LGTM. Yasuhito, if you want to review my changes to
> your patch and perhaps address the issue in the last paragraph then I can
> commit your patch to swig-py3. I'll also start working more on the swig-py3
> branch to get it to the point that we can actually get this branch merged
> into trunk finally.
Great. :-)
Cheers,
Daniel
Received on 2018-12-16 17:28:20 CET