-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
[fix][docs] Remove redudant steps in docs gen #18132
Conversation
Signed-off-by: Mercurio <[email protected]>
Signed-off-by: Mercurio <[email protected]>
…lient,pulsar,pulsar-perf Signed-off-by: Mercurio <[email protected]>
Signed-off-by: Mercurio <[email protected]>
Signed-off-by: Mercurio <[email protected]>
Signed-off-by: Mercurio <[email protected]>
LGTM PTAL @urfreespace @tisonkun thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution! Comments inline.
"reference-cli-tools", | ||
{ | ||
"type": "link", | ||
"href": "https://pulsar.apache.org/reference", | ||
"label": "Pulsar configuration" | ||
}, | ||
"reference-configuration", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After #18101, I think both "reference-cli-tools" and this new "reference-configuration" page redirect to http://pulsar.apache.org/reference?
But we can improve the docsify side later...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anonymitaet suggests that we should make it a Markdown page instead of a direct link, so it's changed back here.
for (String s : cmdObj.jcommander.getCommands().keySet()) { | ||
sb.append("* `").append(s).append("`\n"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share a preview of diff how these lines affect the final result?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Co-authored-by: tison <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
@SignorMercurio feel free to ping me to merge if all tests pass. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #18132 +/- ##
=============================================
+ Coverage 34.91% 45.99% +11.08%
- Complexity 5707 17667 +11960
=============================================
Files 607 1574 +967
Lines 53396 128502 +75106
Branches 5712 14144 +8432
=============================================
+ Hits 18644 59107 +40463
- Misses 32119 63283 +31164
- Partials 2633 6112 +3479
Flags with carried forward coverage won't be shown. Click here to find out more.
|
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
Motivation
There are some redundant information in current pulsar docs, as pointed out by @Anonymitaet. This PR aims to fix those.
Modifications
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
doc
doc-required
doc-not-needed
doc-complete
Matching PR in forked repository
PR in forked repository: SignorMercurio#5