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