I have a number of comments:
> Log:
> Merge Ruby SWIG-based bindings patch.
> http://pub.cozmixng.org/~kou/diff/svn-add-ruby-20050216.diff
>
...
> * subversion/bindings/swig/core.i:
> added SWIG/Ruby related codes.
> swap order of %include and #include.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
No, you didn't do this. So don't include it in the log message! Ditto for
all the other occurrences of it.
> * subversion/bindings/swig/ruby/test/my-assertions.rb: [NEW]
> original assertions for test.
>
> * subversion/bindings/swig/ruby/test/util.rb: [NEW] utility
> methods for test.
>
> * subversion/bindings/swig/ruby/test/test_client.rb: [NEW]
> tests for svn/client.rb.
>
> * subversion/bindings/swig/ruby/test/run-test.rb: [NEW]
> test running script.
>
> * subversion/bindings/swig/ruby/test/test_core.rb: [NEW]
> tests for svn/core.rb.
>
> * subversion/bindings/swig/ruby/test/test_fs.rb: [NEW]
> tests for svn/fs.rb.
>
> * subversion/bindings/swig/ruby/test/test_repos.rb: [NEW]
> tests for svn/repos.rb.
>
> * subversion/bindings/swig/ruby/test/test_delta.rb: [NEW]
> tests for svn/delta.rb.
>
> * subversion/bindings/swig/ruby/test/test_info.rb: [NEW]
> tests for svn/info.rb.
>
> * subversion/bindings/swig/ruby/test/test_util.rb: [NEW]
> tests for svn/util.rb.
>
> * subversion/bindings/swig/ruby/svn/util.rb: [NEW]
> utilities for svn/*.rb.
We don't have a standard of using [NEW] - instead we just say "New file." In
fact that is *all* you need to say - no need to restate what is obvious from
the file name anyway.
> * subversion/bindings/swig/ruby/svn/ext/*.rb: [NEW]
> dummy for svn/ext/_*.so.
Do not use wildcards! (this is documented in HACKING)
Also, why are these present for only three of the _*.so modules?
> * subversion/bindings/swig/ruby/svn/client.rb: [NEW]
> wrapper of svn/ext/_client.so for easy to use.
Not a big deal, but "for easy to use" doesn't actually make sense. "for ease
of use" would be correct, but as mentioned above, a simple "New file" would
have been fine.
Also, please use a capital letter at the beginning of a sentence or phrase.
...
> Modified: branches/ruby/subversion/bindings/swig/svn_client.i
> Url:
> http://svn.collab.net/viewcvs/svn/branches/ruby/subversion/bindings/swig/svn_client.i?view=diff&rev=13034&p1=branches/ruby/subversion/bindings/swig/svn_client.i&r1=13033&p2=branches/ruby/subversion/bindings/swig/svn_client.i&r2=13034
> ==============================================================================
> --- branches/ruby/subversion/bindings/swig/svn_client.i (original) +++
> branches/ruby/subversion/bindings/swig/svn_client.i Tue Feb 15 21:24:45
> 2005
...
> @@ -280,6 +322,45 @@
> argvi++;
> }
>
> +%runtime %{
> + #include <apr.h>
> + #include <apr_pools.h>
> +
> + static apr_pool_t *
> + _svn_client_pool(void)
> + {
> + static apr_pool_t *__svn_client_pool = NULL;
> + if (!__svn_client_pool) {
> + apr_pool_create(&__svn_client_pool, NULL);
> + }
> + return __svn_client_pool;
> + }
> +
> + static apr_pool_t *
> + _svn_client_config_pool(void)
> + {
> + static apr_pool_t *__svn_client_config_pool = NULL;
> + if (!__svn_client_config_pool) {
> + apr_pool_create(&__svn_client_config_pool, _svn_client_pool());
> + }
> + return __svn_client_config_pool;
> + }
> +%}
This will affect all languages, not just Ruby!!!
> Modified: branches/ruby/subversion/bindings/swig/svn_delta.i
> Url:
> http://svn.collab.net/viewcvs/svn/branches/ruby/subversion/bindings/swig/svn_delta.i?view=diff&rev=13034&p1=branches/ruby/subversion/bindings/swig/svn_delta.i&r1=13033&p2=branches/ruby/subversion/bindings/swig/svn_delta.i&r2=13034
> ==============================================================================
> --- branches/ruby/subversion/bindings/swig/svn_delta.i (original) +++
> branches/ruby/subversion/bindings/swig/svn_delta.i Tue Feb 15 21:24:45
> 2005
> @@ -82,6 +89,7 @@
> #include "swigutil_rb.h"
> #endif
> %}
> +%include svn_delta.h
>
> %include svn_delta.h
>
Bad merge!!!
> Modified: branches/ruby/subversion/bindings/swig/svn_ra.i
> Url:
> http://svn.collab.net/viewcvs/svn/branches/ruby/subversion/bindings/swig/svn_ra.i?view=diff&rev=13034&p1=branches/ruby/subversion/bindings/swig/svn_ra.i&r1=13033&p2=branches/ruby/subversion/bindings/swig/svn_ra.i&r2=13034
> ==============================================================================
> --- branches/ruby/subversion/bindings/swig/svn_ra.i (original) +++
> branches/ruby/subversion/bindings/swig/svn_ra.i Tue Feb 15 21:24:45 2005
> @@
> -115,9 +115,16 @@ #include "swigutil_rb.h"
> #endif
> %}
> +%include svn_ra.h
>
> %include svn_ra.h
>
Bad merge!!!
Max.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Feb 17 13:09:31 2005