Re: [PATCH] Compressed streams (take 2)

Eric Dorland wrote:

>Alrighty, attached is the compressed stream code with an extra large
>test case, which will cause the read code to use more than one buffer
>full of data and also addresses all the other points that were
>raised. It's kind of a big patch since the test case had to be *big* :)
>

    * Look at libsvn_delta/random-test.c. We generate random bytes there
      and do provide a way to reproduce the test. Don't, and I mean do
      *not*, hard-code "random" bytes in the test, because then you're
      not testing on random data, you're testing on hard-coded data.

    * What's with all the indentation changes in configure.in? 90% of
      your changes there are just indentation. Lose them.

    * You're putting tabs in the sources. We don't use tabs in the
      sources. Read HACKING about how our stomachs turn at tabs in the
      sources.

    * Please don't define the symbol "ZLIB". Instead, use something
      that's namespace-protected; for instance, "SVN_HAVE_ZLIB" or
      something like that would be much better.

    * In the docstring for svn_stream_compressed in svn_io.h, you
      explicitly mention zlib compression. I think the use of ZLIB is an
      implementation detail, and doesn't have to be mentioned in the
      docstring.

    * Please add a comment about what ZBUFFER_SIZE is for.

-- 
Brane Čibej   <brane_at_xbc.nu>   http://www.xbc.nu/brane/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org