Re: svn commit: r1755486 - in /subversion/trunk/subversion: include/svn_io.h libsvn_fs/fs-loader.c libsvn_repos/config_pool.c libsvn_subr/io.c libsvn_subr/stream.c tests/libsvn_fs/fs-test.c tests/libsvn_subr/stream-test.c

On 17 August 2016 at 01:47, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> Belated review:
>
> ivan_at_apache.org wrote on Mon, Aug 08, 2016 at 10:32:29 -0000:
>> +/** Return in @a *checksum the checksum of type @a kind of @a stream.
>> + *
>
> Have the docstring state whether STREAM is read from, written to, or
> both? (I know the answer; I am just proposing the docstring should
> state it explicitly.)
>
>> + * Stream will be closed before this function returns (regardless of
>> + * the result, or any possible error).
>> + *
>
> s/Stream/@a stream/
>
> Or: s/Stream/The stream/
>
I've tried to improve docstring in r1756647.

>> + * Use @a scratch_pool for temporary allocations and @a result_pool
>> + * to allocate @a *checksum.
>> + *
>> + * @since New in 1.10.
>> + */
>> +svn_error_t *
>> +svn_stream_checksum(svn_checksum_t **checksum,
>
> This interface seems very similar to svn_stream_checksummed(), both in
> name and in functionality.
>
I don't see problem here, but I could suggest name it as
svn_stream_contents_checksum(). What do you think?

-- 
Ivan Zhakov