You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2012/10/16 00:55:14 UTC

API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> Author: stefan2
> Date: Sun Jun 10 21:34:21 2012
> New Revision: 1348666
> 
> URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> Log:
> When handing out node contents, the delta streams don't need
> to calculate MD5 checksums as the result will not be used and the
> check would be redundant even if it were made.
> 
> Thus, rev the svn_txdelta API to calculate the checksum only
> upon specific request and update all callers to use the new API.
> 
...
> Modified: subversion/trunk/subversion/include/svn_delta.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_delta.h (original)
> +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10 21:34:21 2012
> @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> + * If @a calculate_checksum
> + * is set, you may call @ref svn_txdelta_md5_digest to get an MD5 checksum
> + * for @a target.
>   *
>   * Do any necessary allocation in a sub-pool of @a pool.
> + *
> + * @since New in 1.8.
> + */
> +void
> +svn_txdelta2(svn_txdelta_stream_t **stream,
> +             svn_stream_t *source,
> +             svn_stream_t *target,
> +             svn_boolean_t calculate_checksum,

As of today, only svn_txdelta() and window-test.c pass TRUE for @a
calculate_checksum.  Do we anticipate needing to pass TRUE For it in any
new code?

If not, we could take this opportunity to remove this parameter from the
public API (it's a trivial patch).

> +             apr_pool_t *pool);
> +
> +/** Similar to svn_txdelta2 but always calculating the target checksum.
> + *
> + * @deprecated Provided for backward compatibility with the 1.7 API.
>   */
> +SVN_DEPRECATED
>  void
>  svn_txdelta(svn_txdelta_stream_t **stream,
>              svn_stream_t *source,
> 

Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sun, Oct 21, 2012 at 19:43:16 +0200:
> On Sun, Oct 21, 2012 at 5:55 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> 
> > Stefan Fuhrmann wrote on Sat, Oct 20, 2012 at 15:14:11 +0200:
> > > On Thu, Oct 18, 2012 at 10:22 PM, Daniel Shahaf <d.s@daniel.shahaf.name
> > >wrote:
> > >
> > > > Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200:
> > > > > On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <
> > d.s@daniel.shahaf.name
> > > > >wrote:
> > > > >
> > > > > > stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > > > > > > Author: stefan2
> > > > > > > Date: Sun Jun 10 21:34:21 2012
> > > > > > > New Revision: 1348666
> > > > > > >
> > > > > > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > > > > > > Log:
> > > > > > > When handing out node contents, the delta streams don't need
> > > > > > > to calculate MD5 checksums as the result will not be used and the
> > > > > > > check would be redundant even if it were made.
> > > > > > >
> > > > > > > Thus, rev the svn_txdelta API to calculate the checksum only
> > > > > > > upon specific request and update all callers to use the new API.
> > > > > > >
> > > > > > ...
> > > > > > > Modified: subversion/trunk/subversion/include/svn_delta.h
> > > > > > > URL:
> > > > > >
> > > >
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> > > > > > >
> > > > > >
> > > >
> > ==============================================================================
> > > > > > > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > > > > > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10
> > > > 21:34:21
> > > > > > 2012
> > > > > > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > > > > > > + * If @a calculate_checksum
> > > > > > > + * is set, you may call @ref svn_txdelta_md5_digest to get an
> > MD5
> > > > > > checksum
> > > > > > > + * for @a target.
> > > > > > >   *
> > > > > > >   * Do any necessary allocation in a sub-pool of @a pool.
> > > > > > > + *
> > > > > > > + * @since New in 1.8.
> > > > > > > + */
> > > > > > > +void
> > > > > > > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > > > > > > +             svn_stream_t *source,
> > > > > > > +             svn_stream_t *target,
> > > > > > > +             svn_boolean_t calculate_checksum,
> > > > > >
> > > > > > As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> > > > > > calculate_checksum.  Do we anticipate needing to pass TRUE For it
> > in
> > > > any
> > > > > > new code?
> > > > > >
> > > > >
> > > > > No, I don't anticipate extra need for that parameter
> > > > > neither do I feel that it is useless. How would you
> > > > > implement svn_txdelta() without that parameter?
> > > >
> > >
> > > Hm .. wrong question :/
> > >
> > > I would remove the parameter from the public API only.  Internally we
> > > > will define both svn_txdelta() and svn_txdelta2() as one-line wrappers
> > > > around some third function which does take an 'svn_boolean_t
> > > > calculate_checksum' parameter.
> > > >
> > >
> > > That would only be o.k., if that internal function was internal
> > > to libsvn_delta. But we would need to call it from other libs
> > > which I see as a strong indicator that others might want this
> > > functionality, too.
> >
> > We wouldn't need to call the internal function from other libs.
> >
> > static void
> > foo_internal(svn_txdelta_stream_t **stream,
> >              svn_stream_t *source,
> >              svn_stream_t *target,
> >              svn_boolean_t calculate_checksum,
> >              apr_pool_t *pool);
> >
> > void
> > svn_txdelta2(stream, source, target, pool) {
> >   foo_internal(stream, source, target, FALSE, pool);
> > }
> >
> > void
> > svn_txdelta(stream, source, target, pool) {
> >   foo_internal(stream, source, target, TRUE, pool);
> > }
> >
> 
> We seem to talk past each other here. My problem
> is that the BDB / FSFS libs would need to call the
> *deprecated* svn_txdelta because they need the MD5
> checksum.

Odd.  I thought that there were no calls to svn_txdelta() left in the
codebase, and that all calls to svn_txdelta2() passed FALSE for the new
boolean parameter.

Anyway, shrug.  I think this thread is too long for the prospective
benefit (one fewer boolean parameter on a 5-ary function).

Cheers

Daniel

Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Oct 21, 2012 at 5:55 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> Stefan Fuhrmann wrote on Sat, Oct 20, 2012 at 15:14:11 +0200:
> > On Thu, Oct 18, 2012 at 10:22 PM, Daniel Shahaf <d.s@daniel.shahaf.name
> >wrote:
> >
> > > Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200:
> > > > On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <
> d.s@daniel.shahaf.name
> > > >wrote:
> > > >
> > > > > stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > > > > > Author: stefan2
> > > > > > Date: Sun Jun 10 21:34:21 2012
> > > > > > New Revision: 1348666
> > > > > >
> > > > > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > > > > > Log:
> > > > > > When handing out node contents, the delta streams don't need
> > > > > > to calculate MD5 checksums as the result will not be used and the
> > > > > > check would be redundant even if it were made.
> > > > > >
> > > > > > Thus, rev the svn_txdelta API to calculate the checksum only
> > > > > > upon specific request and update all callers to use the new API.
> > > > > >
> > > > > ...
> > > > > > Modified: subversion/trunk/subversion/include/svn_delta.h
> > > > > > URL:
> > > > >
> > >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> > > > > >
> > > > >
> > >
> ==============================================================================
> > > > > > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > > > > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10
> > > 21:34:21
> > > > > 2012
> > > > > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > > > > > + * If @a calculate_checksum
> > > > > > + * is set, you may call @ref svn_txdelta_md5_digest to get an
> MD5
> > > > > checksum
> > > > > > + * for @a target.
> > > > > >   *
> > > > > >   * Do any necessary allocation in a sub-pool of @a pool.
> > > > > > + *
> > > > > > + * @since New in 1.8.
> > > > > > + */
> > > > > > +void
> > > > > > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > > > > > +             svn_stream_t *source,
> > > > > > +             svn_stream_t *target,
> > > > > > +             svn_boolean_t calculate_checksum,
> > > > >
> > > > > As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> > > > > calculate_checksum.  Do we anticipate needing to pass TRUE For it
> in
> > > any
> > > > > new code?
> > > > >
> > > >
> > > > No, I don't anticipate extra need for that parameter
> > > > neither do I feel that it is useless. How would you
> > > > implement svn_txdelta() without that parameter?
> > >
> >
> > Hm .. wrong question :/
> >
> > I would remove the parameter from the public API only.  Internally we
> > > will define both svn_txdelta() and svn_txdelta2() as one-line wrappers
> > > around some third function which does take an 'svn_boolean_t
> > > calculate_checksum' parameter.
> > >
> >
> > That would only be o.k., if that internal function was internal
> > to libsvn_delta. But we would need to call it from other libs
> > which I see as a strong indicator that others might want this
> > functionality, too.
>
> We wouldn't need to call the internal function from other libs.
>
> static void
> foo_internal(svn_txdelta_stream_t **stream,
>              svn_stream_t *source,
>              svn_stream_t *target,
>              svn_boolean_t calculate_checksum,
>              apr_pool_t *pool);
>
> void
> svn_txdelta2(stream, source, target, pool) {
>   foo_internal(stream, source, target, FALSE, pool);
> }
>
> void
> svn_txdelta(stream, source, target, pool) {
>   foo_internal(stream, source, target, TRUE, pool);
> }
>

We seem to talk past each other here. My problem
is that the BDB / FSFS libs would need to call the
*deprecated* svn_txdelta because they need the MD5
checksum.

-- Stefan^2.

-- 
*

Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*

Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Sat, Oct 20, 2012 at 15:14:11 +0200:
> On Thu, Oct 18, 2012 at 10:22 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> 
> > Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200:
> > > On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <d.s@daniel.shahaf.name
> > >wrote:
> > >
> > > > stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > > > > Author: stefan2
> > > > > Date: Sun Jun 10 21:34:21 2012
> > > > > New Revision: 1348666
> > > > >
> > > > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > > > > Log:
> > > > > When handing out node contents, the delta streams don't need
> > > > > to calculate MD5 checksums as the result will not be used and the
> > > > > check would be redundant even if it were made.
> > > > >
> > > > > Thus, rev the svn_txdelta API to calculate the checksum only
> > > > > upon specific request and update all callers to use the new API.
> > > > >
> > > > ...
> > > > > Modified: subversion/trunk/subversion/include/svn_delta.h
> > > > > URL:
> > > >
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> > > > >
> > > >
> > ==============================================================================
> > > > > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > > > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10
> > 21:34:21
> > > > 2012
> > > > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > > > > + * If @a calculate_checksum
> > > > > + * is set, you may call @ref svn_txdelta_md5_digest to get an MD5
> > > > checksum
> > > > > + * for @a target.
> > > > >   *
> > > > >   * Do any necessary allocation in a sub-pool of @a pool.
> > > > > + *
> > > > > + * @since New in 1.8.
> > > > > + */
> > > > > +void
> > > > > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > > > > +             svn_stream_t *source,
> > > > > +             svn_stream_t *target,
> > > > > +             svn_boolean_t calculate_checksum,
> > > >
> > > > As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> > > > calculate_checksum.  Do we anticipate needing to pass TRUE For it in
> > any
> > > > new code?
> > > >
> > >
> > > No, I don't anticipate extra need for that parameter
> > > neither do I feel that it is useless. How would you
> > > implement svn_txdelta() without that parameter?
> >
> 
> Hm .. wrong question :/
> 
> I would remove the parameter from the public API only.  Internally we
> > will define both svn_txdelta() and svn_txdelta2() as one-line wrappers
> > around some third function which does take an 'svn_boolean_t
> > calculate_checksum' parameter.
> >
> 
> That would only be o.k., if that internal function was internal
> to libsvn_delta. But we would need to call it from other libs
> which I see as a strong indicator that others might want this
> functionality, too.

We wouldn't need to call the internal function from other libs.

static void
foo_internal(svn_txdelta_stream_t **stream,
             svn_stream_t *source,
             svn_stream_t *target,
             svn_boolean_t calculate_checksum,
             apr_pool_t *pool);

void
svn_txdelta2(stream, source, target, pool) {
  foo_internal(stream, source, target, FALSE, pool);
}

void
svn_txdelta(stream, source, target, pool) {
  foo_internal(stream, source, target, TRUE, pool);
}


Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Thu, Oct 18, 2012 at 10:22 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200:
> > On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <d.s@daniel.shahaf.name
> >wrote:
> >
> > > stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > > > Author: stefan2
> > > > Date: Sun Jun 10 21:34:21 2012
> > > > New Revision: 1348666
> > > >
> > > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > > > Log:
> > > > When handing out node contents, the delta streams don't need
> > > > to calculate MD5 checksums as the result will not be used and the
> > > > check would be redundant even if it were made.
> > > >
> > > > Thus, rev the svn_txdelta API to calculate the checksum only
> > > > upon specific request and update all callers to use the new API.
> > > >
> > > ...
> > > > Modified: subversion/trunk/subversion/include/svn_delta.h
> > > > URL:
> > >
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> > > >
> > >
> ==============================================================================
> > > > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10
> 21:34:21
> > > 2012
> > > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > > > + * If @a calculate_checksum
> > > > + * is set, you may call @ref svn_txdelta_md5_digest to get an MD5
> > > checksum
> > > > + * for @a target.
> > > >   *
> > > >   * Do any necessary allocation in a sub-pool of @a pool.
> > > > + *
> > > > + * @since New in 1.8.
> > > > + */
> > > > +void
> > > > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > > > +             svn_stream_t *source,
> > > > +             svn_stream_t *target,
> > > > +             svn_boolean_t calculate_checksum,
> > >
> > > As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> > > calculate_checksum.  Do we anticipate needing to pass TRUE For it in
> any
> > > new code?
> > >
> >
> > No, I don't anticipate extra need for that parameter
> > neither do I feel that it is useless. How would you
> > implement svn_txdelta() without that parameter?
>

Hm .. wrong question :/

I would remove the parameter from the public API only.  Internally we
> will define both svn_txdelta() and svn_txdelta2() as one-line wrappers
> around some third function which does take an 'svn_boolean_t
> calculate_checksum' parameter.
>

That would only be o.k., if that internal function was internal
to libsvn_delta. But we would need to call it from other libs
which I see as a strong indicator that others might want this
functionality, too.

-- Stefan^2.

-- 
*

Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*

Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Stefan Fuhrmann wrote on Thu, Oct 18, 2012 at 02:56:15 +0200:
> On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> 
> > stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > > Author: stefan2
> > > Date: Sun Jun 10 21:34:21 2012
> > > New Revision: 1348666
> > >
> > > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > > Log:
> > > When handing out node contents, the delta streams don't need
> > > to calculate MD5 checksums as the result will not be used and the
> > > check would be redundant even if it were made.
> > >
> > > Thus, rev the svn_txdelta API to calculate the checksum only
> > > upon specific request and update all callers to use the new API.
> > >
> > ...
> > > Modified: subversion/trunk/subversion/include/svn_delta.h
> > > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> > >
> > ==============================================================================
> > > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10 21:34:21
> > 2012
> > > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > > + * If @a calculate_checksum
> > > + * is set, you may call @ref svn_txdelta_md5_digest to get an MD5
> > checksum
> > > + * for @a target.
> > >   *
> > >   * Do any necessary allocation in a sub-pool of @a pool.
> > > + *
> > > + * @since New in 1.8.
> > > + */
> > > +void
> > > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > > +             svn_stream_t *source,
> > > +             svn_stream_t *target,
> > > +             svn_boolean_t calculate_checksum,
> >
> > As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> > calculate_checksum.  Do we anticipate needing to pass TRUE For it in any
> > new code?
> >
> 
> No, I don't anticipate extra need for that parameter
> neither do I feel that it is useless. How would you
> implement svn_txdelta() without that parameter?
> 

I would remove the parameter from the public API only.  Internally we
will define both svn_txdelta() and svn_txdelta2() as one-line wrappers
around some third function which does take an 'svn_boolean_t
calculate_checksum' parameter.

Re: API simplification opportunity Re: svn commit: r1348666 - in /subversion/trunk/subversion: include/ libsvn_delta/ libsvn_fs_fs/ libsvn_ra/ tests/libsvn_delta/

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Tue, Oct 16, 2012 at 12:55 AM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> stefan2@apache.org wrote on Sun, Jun 10, 2012 at 21:34:22 -0000:
> > Author: stefan2
> > Date: Sun Jun 10 21:34:21 2012
> > New Revision: 1348666
> >
> > URL: http://svn.apache.org/viewvc?rev=1348666&view=rev
> > Log:
> > When handing out node contents, the delta streams don't need
> > to calculate MD5 checksums as the result will not be used and the
> > check would be redundant even if it were made.
> >
> > Thus, rev the svn_txdelta API to calculate the checksum only
> > upon specific request and update all callers to use the new API.
> >
> ...
> > Modified: subversion/trunk/subversion/include/svn_delta.h
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_delta.h?rev=1348666&r1=1348665&r2=1348666&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/include/svn_delta.h (original)
> > +++ subversion/trunk/subversion/include/svn_delta.h Sun Jun 10 21:34:21
> 2012
> > @@ -367,10 +367,26 @@ svn_txdelta_md5_digest(svn_txdelta_strea
> > + * If @a calculate_checksum
> > + * is set, you may call @ref svn_txdelta_md5_digest to get an MD5
> checksum
> > + * for @a target.
> >   *
> >   * Do any necessary allocation in a sub-pool of @a pool.
> > + *
> > + * @since New in 1.8.
> > + */
> > +void
> > +svn_txdelta2(svn_txdelta_stream_t **stream,
> > +             svn_stream_t *source,
> > +             svn_stream_t *target,
> > +             svn_boolean_t calculate_checksum,
>
> As of today, only svn_txdelta() and window-test.c pass TRUE for @a
> calculate_checksum.  Do we anticipate needing to pass TRUE For it in any
> new code?
>

No, I don't anticipate extra need for that parameter
neither do I feel that it is useless. How would you
implement svn_txdelta() without that parameter?


> If not, we could take this opportunity to remove this parameter from the
> public API (it's a trivial patch).
>

If we can do without, I'd say let's lose the parameter
and make old and new API only differ in behavior.


> > +             apr_pool_t *pool);
> > +
> > +/** Similar to svn_txdelta2 but always calculating the target checksum.
> > + *
> > + * @deprecated Provided for backward compatibility with the 1.7 API.
> >   */
> > +SVN_DEPRECATED
> >  void
> >  svn_txdelta(svn_txdelta_stream_t **stream,
> >              svn_stream_t *source,
> >
>

-- Stefan^2.

-- 
*

Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*