Hi Pavel,
Though this is an implementation only package, it could use a few more
comments
to support maintainers. In the package.html, a couple of hints about
what classes are
the external interface would be useful and an example of use in Encode
and/or Decode
would help a lot.
The tests use randomness and should have @key randomness in the tests.
To allow reproducibility, the tests should print the seeds used and be
able to be restarted
with a seed. The testlibrary has a convenient factory for Random that
logs the seed.
See jdk.testlibrary.RandomFactory
* @library /lib/testlibrary/
* @build jdk.testlibrary.RandomFactory
and use it as:
private static final Random random = RandomFactory.getRandom();
IntegerReader:
- It would be useful to have a reference to the spec for variable
length integers: i.e. rfc 7541 5.1 Integer Representation
- Line 129, 53: inconsistent IAE's in some the valid condition is
included in the exception message,
but in others the *in*valid condition is included but without any
indication of which is which.
- line 109/110: style put 'while' on same line as '}' from do so it
does not look like a separate statement.
IntegerWriter:
- line 40: perhaps 'value' or 'v' would be more readable than 'i' which
is often thought of as a local value.
Roger
p.s. RandomFactory needs to be moved/copied to the top level test
library and the overlap
with test/lib/Utils.getRandomInstance resolved.
On 4/6/2016 2:00 PM, Pavel Rappo wrote:
Hi,
Could you please review my change for JDK-8153353?
http://cr.openjdk.java.net/~prappo/8153353/webrev.00/
This is an implementation of HPACK (Header Compression for HTTP/2) [1]
Internal API classes are (package sun.net.httpclient.hpack):
Encoder, Decoder and DecodingCallback
Example of their usage can be seen in the RFR for HTTP/2 implementation
published previously today [2, 3].
Thanks,
-Pavel
--------------------------------------------------------------------------------
[1] https://tools.ietf.org/html/rfc7541
[2]
http://cr.openjdk.java.net/~michaelm/8087124/webrev.1/src/java.httpclient/share/classes/java/net/http/Http2Connection.java.html
304:306
[3]
http://cr.openjdk.java.net/~michaelm/8087124/webrev.1/src/java.httpclient/share/classes/java/net/http/Http2Connection.java.html
682:695
