[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

patch API review

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Wed, 28 Apr 2010 10:05:58 -0500

I'm looking at adding support for the patch client API to JavaHL, but in
doing so, I have a few questions about the API itself. I'm pretty late the
game, so these questions might already be answered. If so, please point me
to the relevant thread(s).

For reference, here's the public API:

svn_error_t *
svn_client_patch(const char *abs_patch_path,
                 const char *local_abspath,
                 svn_boolean_t dry_run,
                 int strip_count,
                 svn_boolean_t reverse,
                 const apr_array_header_t *include_patterns,
                 const apr_array_header_t *exclude_patterns,
                 apr_hash_t **patched_tempfiles,
                 apr_hash_t **reject_tempfiles,
                 svn_boolean_t ignore_whitespaces,
                 svn_client_ctx_t *ctx,
                 apr_pool_t *result_pool,
                 apr_pool_t *scratch_pool);

Firstly, the first parameter should probably be 'patch_abspath', which
follows convention with other variable names. That being said, is there a
reason why the patch needs to be a file, instead of a stream? Using a
stream seems much more flexible for the long-term evolution of the API.

Despite my best efforts, I still haven't been able to fully grok the
patched_tempfiles and reject_tempfiles parameters. However, if they are
truly output parameters, they should be at the front of the parameter list,
not hidden in the middle somewhere. Also, I know that the API uses
notifications, but it might also be useful to return the _tempfile
parameters through a callback mechanism, rather than as (arbitrarily large)
hashes.

Anyway, those are my thoughts. I'd go ahead and make these changes myself,
but as I mentioned before, I'm not sure if there are Deep reasons for the
current API.

Thanks,
-Hyrum
Received on 2010-04-28 17:06:36 CEST

This is an archived mail posted to the Subversion Dev mailing list.