The ServerImpl import was removed because it wasn't used as far as
checkstyle was able to determine. However, it was being used to resolve
JavaDoc references. Instead of re-adding the import just make the
reference fully qualified to prevent the two systems from continuing to
disagree on whether it is needed.
The synchronization was simply using the wrong object, so no
synchronization was actually occurring. In addition, reference of cached
outside of synchronized block could permit using a partially constructed
cached object, as the reference may be set before instantiation
completes.
goingAway() is called before onGoAwayRead() in Netty:
b7f57223c1/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java (L521)
The test before checked that the stream went away, but not that the
GOAWAY code influenced our Status, as UNAVAILABLE is the default
internally.
The UNAVAILABLE default has also been changed to include a message so
that we can determine where the Status came from in case it is triggered
again in the future.
We were seeing errors on Travis like:
> Process 'Gradle Test Executor 2' finished with non-zero exit value 137
That doesn't make much sense, other than maybe the OOM killer killing
our processes. Turning off parallel execution seemed to fix the problem,
so we'll just assume memory was the actual problem and doing fewer
things in parallel reduces our maximum memory usage.
Travis documentation seems to agree with that being a likely cause:
http://docs.travis-ci.com/user/common-build-problems/#My-build-script-is-killed-without-any-error
readlink -f allows the script to be run via a symlink yet still
function. However, OS X doesn't have the -f flag. We don't really care
too much about symlinking to the scripts, so just drop readlink -f.
Parallel doesn't mix well with deployment, so if that becomes part of
Travis, we would need to pass -Dorg.gradle.parallel=false to the gradle
command line. But otherwise parallel builds have been solid.
Failing disabled by default, but setting checkstyle.ignoreFailures=false
in ~/.gradle/gradle.properties enables failing. Travis will always run
with such failing enabled.
Also add OkHttpReadableBuffer unit test, and increase the test string
length to expose the bug.
Also use array equality assertions in the base test, instead of
comparisons whose return value is discarded.
Fixes#231.
For code style, we either need a comment describing why an exception is
ignored or to actually handle the exception. In this case there doesn't
seem to be a strong reason to ignore the exception, but it isn't all
that important either, so just log at INFO.
When the description is null, the exception message would be in the form
of "INTERNAL: null", which isn't very attractive, and makes it seem like
an error in itself. If the description is null, just use "INTERNAL".
The checkstyle.xml is a slightly modified version of the upstream Google
checkstyle configuration. All changes have comment describing them.
Lots of warnings were corrected. Examples is the only project that has
warnings still, as the necessary changes require some thought.
-Xlint:-options is not available on some earlier JDK 7s, but won't fail
if unsupported. It prevents the warning wanting bootclasspath specified
since target/source is 1.6.
This allows sooner delivery of errors. We never needed to stop delivery
for unexpected EOS, but instead the application would have been required
to request() another message before delivering. Stalling MessageDeframer
sooner removes the need for the application to request another message
before noticing that the buffers are empty.
We are already specifying to javac that our code is UTF-8, but we also
need to specify to JavaDoc, even though we don't have any non-ASCII
characters in JavaDoc comments.
Tested with LC_ALL=C
Eric Anderson
Xudong Ma
nmittler
zhangkun83
Thomas ten Cate
Louis Ryan
Jakob Buchgraber
