8000 KAFKA-18983 Ensure all README.md(s) are mentioned by the root README.md by johnny94 · Pull Request #19420 · apache/kafka · GitHub
[go: up one dir, main page]
Skip to content

KAFKA-18983 Ensure all README.md(s) are mentioned by the root README.md #19420

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Apr 15, 2025
Merged

KAFKA-18983 Ensure all README.md(s) are mentioned by the root README.md #19420

merged 5 commits into from
Apr 15, 2025

Conversation

johnny94
Copy link
Contributor
@johnny94 johnny94 commented Apr 9, 2025 •
edited by chia7712
Loading

There are few README not added because I am not sure if they need to be mentioned in root README.

./test-common/test-common-internal-api/src/main/java/org/apache/kafka/common/test/api/README.md
./storage/src/test/java/org/apache/kafka/tiered/storage/README.md
./.github/workflows/README.md
./raft/README.md
./committer-tools/README.md

Reviewers: Ken Huang s7133700@gmail.com, TengYao Chi kitingiao@gmail.com, PoAn Yang payang@apache.org, Jhen-Yung Hsu jhenyunghsu@gmail.com, Chia-Ping Tsai chia7712@gmail.com

@github-actions github-actions bot added triage PRs from the community docs tools docker Official Docker image small Small PRs labels Apr 9, 2025
johnny94
johnny94 commented Apr 9, 2025
View reviewed changes
trogdor/README.md
@@ -6,9 +6,9 @@ Trogdor can run benchmarks and other workloads. Trogdor can also inject faults i

Quickstart
=========================================================
First, we want to [start a single-node Kafka cluster in KRaft mode](https://github.com/apache/kafka/blob/trunk/README.md#running-a-kafka-broker-in-kraft-mode)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

running-a-kafka-borker-in-kraft-mode no longer exist by #18301.
Also modified the sentence to align with the PR

frankvicky
frankvicky reviewed Apr 9, 2025
View reviewed changes
Copy link
Contributor
@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@johnny94 : Thanks for the patch.
I have a comment.

docker/README.md Outdated
@@ -31,7 +31,7 @@ Building image and running tests using github actions

- image-type - This is the type of image that we intend to build. This will be dropdown menu type selection in the workflow.
- `jvm` image type is for official docker image (to be hosted on apache/kafka) as described in [KIP-975](https://cwiki.apache.org/confluence/display/KAFKA/KIP-975%3A+Docker+Image+for+Apache+Kafka)
- `native` image type is for graalvm based `native` kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/display/KAFKA/KIP-974%3A+Docker+Image+for+GraalVM+based+Native+Kafka+Broker#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming)
- `native` image type is for graalvm based `native` kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/display/KAFKA/KIP-974%3A+Docker+Image+for+GraalVM+based+Native+Kafka+Broker#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming). Or you can see [native/README.md](native/README.md) for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please replace the KIP link with an immutable link?

Copy link
Contributor Author
@johnny94 johnny94 Apr 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does immutable link mean the link created by the share button?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, for example, the immutable link of KIP-974 is https://cwiki.apache.org/confluence/x/KZizDw

Yunyung
Yunyung reviewed Apr 9, 2025
View reviewed changes
Copy link
Collaborator
@Yunyung Yunyung left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR. I left some comments.

README.md Outdated
10000
@@ -99,6 +99,8 @@ fail due to code changes. You can just run:

./gradlew processMessages processTestMessages

See [Apache Kafka Message Definitions](/clients/src/main/resources/common/message/README.md) for details on Apache Kafka message protocol
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

period

README.md Outdated
@@ -263,10 +267,28 @@ default. See https://www.lightbend.com/blog/scala-inliner-optimizer for more det

See [tests/README.md](tests/README.md).

### Using Trogdor for testing ###

We Trogdor as a test framework for Apache Kafka. You can use it to run benchmarks and other workloads.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We "use" Trogdor?

FrankYang0529
FrankYang0529 reviewed Apr 9, 2025
View reviewed changes
README.md Outdated
@@ -111,6 +113,8 @@ Using docker image:

docker run -p 9092:9092 apache/kafka:3.7.0

See [docker/README.md] for detailed information.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be [docker/README.md](docker/README.md).

m1a2st
m1a2st reviewed Apr 9, 2025
View reviewed changes
Copy link
Collaborator
@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this patch, a little comments

docker/README.md Outdated
@@ -30,8 +30,8 @@ Building image and running tests using github actions
- kafka-url - This is the url to download kafka tarball from. For example kafka tarball url from (https://archive.apache.org/dist/kafka). For building RC image this will be an RC tarball url.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you also update this link to markdown pattern

(https://archive.apache.org/dist/kafka) -> [Kafka archive](https://archive.apache.org/dist/kafka)

docker/README.md Outdated
- `jvm` image type is for official docker image (to be hosted on apache/kafka) as described in [KIP-975](https://cwiki.apache.org/confluence/display/KAFKA/KIP-975%3A+Docker+Image+for+Apache+Kafka)
- `native` image type is for graalvm based `native` kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/display/KAFKA/KIP-974%3A+Docker+Image+for+GraalVM+based+Native+Kafka+Broker#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming)
- `jvm` image type is for official docker image (to be hosted on apache/kafka) as described in [KIP-975](https://cwiki.apache.org/confluence/x/z5izDw)
- `native` image type is for graalvm based `native` kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/x/KZizDw#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming). Or you can see [native/README.md](native/README.md) for more information.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"kafka docker image" could be "Kafka docker image"

@github-actions github-actions bot removed the triage PRs from the community label Apr 10, 2025
frankvicky
frankvicky reviewed Apr 10, 2025
View reviewed changes
docker/README.md Outdated
- `jvm` image type is for official docker image (to be hosted on apache/kafka) as described in [KIP-975](https://cwiki.apache.org/confluence/display/KAFKA/KIP-975%3A+Docker+Image+for+Apache+Kafka)
- `native` image type is for graalvm based `native` kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/display/KAFKA/KIP-974%3A+Docker+Image+for+GraalVM+based+Native+Kafka+Broker#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming)
- `jvm` image type is for official docker image (to be hosted on apache/kafka) as described in [KIP-975](https://cwiki.apache.org/confluence/x/z5izDw)
- `native` image type is for graalvm based `native` Kafka docker image (to be hosted on apache/kafka-native) as described in [KIP-974](https://cwiki.apache.org/confluence/x/KZizDw#KIP974:DockerImageforGraalVMbasedNativeKafkaBroker-ImageNaming). Or you can see [native/README.md](native/README.md) for more information.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we only keep https://cwiki.apache.org/confluence/x/KZizDw ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No preference. Just follow the original link.

Yunyung
Yunyung reviewed Apr 14, 2025
View reviewed changes
README.md Outdated
@@ -99,6 +99,8 @@ fail due to code changes. You can just run:

./gradlew processMessages processTestMessages

See [Apache Kafka Message Definitions](/clients/src/main/resources/common/message/README.md) for details on Apache Kafka message protocol.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there a '/' in '/clients'? Can we remove it?

@chia7712 chia7712 merged commit e9ca0bb into apache:trunk Apr 15, 2025
20 checks passed
ijuma
ijuma reviewed Apr 16, 2025
View reviewed changes
README.md
### Running in Vagrant ###

See [vagrant/README.md](vagrant/README.md).

### Release Kafka ###
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not something a regular person can do and hence it doesn't seem like something we should link from the main readme.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The original idea was to list all README.md files in the root directory, without filtering the content for specific readers. I'm fine to remove the "Release Kafka", and the "Official Documentation" could be removed too as the file "docs/README.md" has nothing to read

@johnny94 johnny94 mentioned this pull request Apr 16, 2025
Merged
chia7712 pushed a commit that referenced this pull request Apr 16, 2025
@johnny94
MINOR: Remove Kafka release section from root README (#19483)
58ad29c 
Addressed the feedback
#19420 (comment)

Reviewers: TengYao Chi <kitingiao@gmail.com>, PoAn Yang
<payang@apache.org>, Chia-Ping Tsai <chia7712@gmail.com>
rreddy-22 pushed a commit to rreddy-22/kafka-rreddy that referenced this pull request Apr 21, 2025
A8D3
@johnny94 @rreddy-22
MINOR: Remove Kafka release section from root README (apache#19483)
6b10181 
Addressed the feedback
apache#19420 (comment)

Reviewers: TengYao Chi <kitingiao@gmail.com>, PoAn Yang
<payang@apache.org>, Chia-Ping Tsai <chia7712@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ijuma ijuma ijuma left review comments

@FrankYang0529 FrankYang0529 FrankYang0529 left review comments

@chia7712 chia7712 chia7712 approved these changes

@Yunyung Yunyung Yunyung left review comments

@m1a2st m1a2st m1a2st left review comments

@frankvicky frankvicky frankvicky approved these changes

Assignees
No one assigned
Labels
ci-approved docker Official Docker image docs small Small PRs tools
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@johnny94 @ijuma @FrankYang0529 @chia7712 @Yunyung @frankvicky @m1a2st @mingyen066
0