KAFKA-12435: Fix javadoc errors #10392
Conversation
| @@ -71,7 +71,7 @@ | |||
| * <p> | |||
| * <pre> | |||
| * +---------------------+ +----------------------+ | |||
| * |COPY_SEGMENT_STARTED |----------->|COPY_SEGMENT_FINISHED | | |||
| * |COPY_SEGMENT_STARTED |----------->|COPY_SEGMENT_FINISHED | | |||
There was a problem hiding this comment.
These were just warnings, but they were the only two warnings in the project, so I fixed them. We just need to XML-escape the > character.
| @@ -237,7 +237,7 @@ private static boolean isRecoverable(final KafkaException uncaughtException) { | |||
| * @throws IllegalStateException if EOS is disabled | |||
| * @throws TaskMigratedException | |||
| */ | |||
| void commitTransaction(final Map<TopicPartition, OffsetAndMetadata> offsets, | |||
| protected void commitTransaction(final Map<TopicPartition, OffsetAndMetadata> offsets, | |||
There was a problem hiding this comment.
Needed this so I could move the subclass to a different package. I think it's still just as obviously inappropriate for users to subclass this class, since it's in the internals package.
| import org.apache.kafka.streams.internals.KeyValueStoreFacade; | ||
| import org.apache.kafka.streams.internals.WindowStoreFacade; | ||
| import org.apache.kafka.streams.test.internal.KeyValueStoreFacade; | ||
| import org.apache.kafka.streams.test.internal.WindowStoreFacade; |
There was a problem hiding this comment.
Javadoc is unable to generate docs for this (TopologyTestDriver) file because it depends on these classes, which are also in test-utils, but are excluded in the build.gradle spec.
I was unable to override the exclusion with a more specific inclusion, so instead I just moved these classes to a different package that matches only the "include" patterns in :streams:test-utils:javadoc.
There was a problem hiding this comment.
Have you checked that the produced javadoc does not include these internal files?
There was a problem hiding this comment.
I didn't before, but I did just check on my latest strategy, and they are not generated.
| @@ -14,7 +14,7 @@ | |||
| * See the License for the specific language governing permissions and | |||
| * limitations under the License. | |||
| */ | |||
| package org.apache.kafka.streams.internals; | |||
| package org.apache.kafka.streams.test.internal; | |||
There was a problem hiding this comment.
It's not great that we use internal here and internals everywhere else. I think you did this to avoid the javadoc exclusion pattern, but...
There was a problem hiding this comment.
Yes, that's exactly right. I agree it's not great. Maybe a better alternative is to just not try to mark these classes as "internal". It should be pretty hard to get confused about their usage, and apparently we have to include them in the Javadoc anyway, so it's probably fine to just move them up a level and drop internal
|
Ok, thanks for the reviews, @ijuma and @chia7712 ! I decided that instead of playing subtle games with the pattern matching and the package name, I'll just make these classes into static inner classes of TopologyTestDriver (their only usage). I double-checked the generated javadoc, and I don't see anything suspicious either in the package tree or in the TopologyTestDriver.html doc. |
| @@ -1622,7 +1622,6 @@ project(':streams:test-utils') { | |||
|
|
|||
| javadoc { | |||
| include "**/org/apache/kafka/streams/test/**" | |||
| exclude "**/org/apache/kafka/streams/internals/**", "**/org/apache/kafka/streams/**/internals/**" | |||
There was a problem hiding this comment.
As the related class have be moved from internals, does it need to remove this exclude rule?
There was a problem hiding this comment.
The files I converted to static inner classes were the only contents of these packages, so they don't exist anymore. I figured I'd go ahead and remove the exclusion, too.
|
Ok, I've tested this fix locally, and it looks like, although we have some flaky test failures, the builds are still functioning. |
There were errors while generating javadoc for the streams:test-utils module because the included TopologyTestDriver imported some excluded classes. This fixes the errors by inlining the previously excluded packages. Reviewers: Chia-Ping Tsai <chia7712@apache.org>, Ismael Juma <ijuma@apache.org>
There were errors while generating javadoc for the streams:test-utils module because the included TopologyTestDriver imported some excluded classes. This fixes the errors by inlining the previously excluded packages. Reviewers: Chia-Ping Tsai <chia7712@apache.org>, Ismael Juma <ijuma@apache.org>
Fixes errors while generating javadoc.
Committer Checklist (excluded from commit message)