On 11/23/18 11:21 AM, Daniel Shahaf wrote:
Thanks for the patch. I'm afraid I'm a bit disoriented, though; could
you kindly clarify a few things?
Is the patch destined for trunk, for branches/swig-py3@HEAD, or for
branches/swig-py3 after a sync-merge from trunk?
It's destined for branches/swig-py3@HEAD, but actually I did sync-merge
from trunk@1846855 and tag 1.11.0, then I confirm via diff summary that
the change in my patches doesn't affect merge.
(I'm sorry, I did it on git,
https://github.com/futatuki/subversion/tree/swig-py3)
Does it replace your
previous patch, or are the two patches meant to be applied on top of
each other?
This is the latter. My secondary patch doesn't replace previous patch,
but assuming first patch applied.
How do we test the patch to see the problem that it
corrects --- I assume that we should just run check-swig-py in a py3
build and see that it has fewer failures with the patch than without; is
that correct?
It is partly correct, because my patch may affect some place where curent
test doesn't check. Actually I run check-swig-py before apply secondary
patch, it returns ok without failure, so I think it doen't test
svn_stream_write() and svn_stream_readline(), though I'm not see whole
of the test.
So I did also bulding subversion/bindings/swig/python/
both of before apply patch and after apply, then diff them, to make sure
the patch doesn't affect other functions other than functions related
to svn_stream_read*() and svn_stream_write().
(I confess I made a diff only for subversion/bindings/swig/python/core.c
on my previous post, and now I found the patches affects
subversion/bindings/swig/python/core.py and
subversion/bindings/swig/python/libsvn/core.py, with incorrect function
annotation for svn_stream_readline() and svn_stream_invoke_readline_fn().)
Of course, I think it is not sufficient and I think it need tests for
svn_stream_readline() and svn_stream_write().
I ran ViewVC as a test for svn_stream_readline(), with branch merged
branch swig-py3 and 1.11.0 tree (for my own convenient),
but I know it is not appropriate here, and it isn't use svn_stream_write().
Thanks,
--
Yasuhito FUTATSUKI
