Skip to content
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.

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

MINOR: Fix newlines not working in LISTENERS_DOC #18388

Conversation

clarkwtc
Copy link
Contributor

@clarkwtc clarkwtc commented Jan 5, 2025

Fix newlines not working with %n in LISTENERS_DOC

Following the Kafka site preview below:

Before
Screenshot 2025-01-05 140840

After
Screenshot 2025-01-05 162419

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@github-actions github-actions bot added triage PRs from the community core Kafka Broker small Small PRs labels Jan 5, 2025
Copy link
Member

@FrankYang0529 FrankYang0529 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 patch. Leave a minor comment.

" <code>CLIENT://0.0.0.0:9092,REPLICATION://localhost:9093</code>%n" +
" <code>PLAINTEXT://127.0.0.1:9092,SSL://[::1]:9092</code>%n";
public static final String LISTENERS_DOC = String.format("Listener List - Comma-separated list of URIs we will listen on and the listener names." +
" If the listener name is not a security protocol, <code>%1$1s</code> must also be set.%n" +
Copy link
Member

Choose a reason for hiding this comment

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

Why using %1$1s? Change to %s can fix the issue on my laptop.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right.
Use %s can work on it.

I fixed it.
Thank you for your comment.

Copy link
Member

@FrankYang0529 FrankYang0529 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the patch.

Copy link
Contributor

@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 @clarkwtc, left a little comment.

Comment on lines 70 to 71
" Listener names and port numbers must be unique unless %n" +
" one listener is an IPv4 address and the other listener is %n" +
Copy link
Contributor

Choose a reason for hiding this comment

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

nit This seems to be a complete sentence. Could you remove the line separator?”

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it is a good point.

I removed it.
Thank you for your comment.

@clarkwtc clarkwtc force-pushed the minor-fix-newline-not-working-in-listeners-doc branch from 1431d45 to 5e9f528 Compare January 5, 2025 08:26
Copy link
Contributor

@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.

LGTM

@github-actions github-actions bot removed the triage PRs from the community label Jan 6, 2025
@chia7712 chia7712 merged commit b9354d6 into apache:trunk Jan 6, 2025
7 of 9 checks passed
chia7712 pushed a commit that referenced this pull request Jan 6, 2025
tedyu pushed a commit to tedyu/kafka that referenced this pull request Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved core Kafka Broker small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants