Skip to content

[fix][doc] Optimize URLs for CLI tools page - #18101

Merged
codelipenghui merged 2 commits into
apache:masterfrom
Anonymitaet:1019
Oct 27, 2022
Merged

[fix][doc] Optimize URLs for CLI tools page#18101
codelipenghui merged 2 commits into
apache:masterfrom
Anonymitaet:1019

Conversation

@Anonymitaet

@Anonymitaet Anonymitaet commented Oct 19, 2022

Copy link
Copy Markdown
Member

This PR:

  1. Removes metadata since it should not be shown on the Reference site

image

  1. Removes broker-tool because [feat][doc] Separate CLI docs for doc gen automation #18051 (comment)

  2. Update URLs to https://pulsar.apache.org/reference to make it more generic and easier to maintain

  • doc

@Anonymitaet Anonymitaet added this to the 2.11.0 milestone Oct 19, 2022
@github-actions github-actions Bot added the doc Your PR contains doc changes, no matter whether the changes are in markdown or code files. label Oct 19, 2022
@Anonymitaet

Copy link
Copy Markdown
Member Author

@SignorMercurio
could you please review this PR from a technical perspective? Thank you!

cc @tisonkun @momo-jun @DaveDuggins @D-2-Ed

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I check the experience from:

All works correctly. Could you share a video to clarify the issue?

Screen.Recording.2022-10-19.at.13.11.46.mov

@SignorMercurio SignorMercurio left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why not use a single link instead of a list of CLI tool?

@Anonymitaet

Copy link
Copy Markdown
Member Author

@SignorMercurio updated as you suggested, PTAL

@Anonymitaet

Copy link
Copy Markdown
Member Author

@tisonkun

Update URLs to https://pulsar.apache.org/reference to make it more generic and easier to maintain

All commands in the green box are on the same page, so I set them all to https://pulsar.apache.org/reference (users can choose versions then) instead of individual links for lower maintenance costs and a temporary workaround for #18051 (comment)
image

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

OK...Although I think it changes several links if users save them in their bookmark previously.

Comments inline.

Comment thread site2/docs/about.md Outdated
:::tip

For how to make contributions to documentation, see [Pulsar Documentation Contribution Guide](https://docs.google.com/document/d/11DTnNPpvcPrebLkMAFcDEIFlD8ARD-k6F-LXoIwdD9Y/edit#).
For how to make contributions to documentation, see [Pulsar Documentation Contribution Guide](../README.md).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't move this README.md file to the site repo.

You can use https://github.com/apache/pulsar/blob/master/site2/README.md as a workaround, which I hope we can move into another doc page and update these links to the new page.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sorry I don't fully understand this comment

Now the doc contribution guide overview is in site2/README.md, and [Pulsar Documentation Contribution Guide](../README.md) redirects users to the correct place.

image

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It works on the GitHub repository but not for the Pulsar website. You can try to preview it and verify if you can jump to the README.md file.

Comment thread site2/docs/reference-cli-bookkeeper.md Outdated
Comment on lines -1 to -5
---
id: reference-cli-bookkeeper
title: Pulsar CLI tools - bookkeeper
sidebar_label: "cli-bookkeeper"
---

@tisonkun tisonkun Oct 19, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do you make these chanes? If you'd like to set up redirections, I suggest:

````mdx-code-block
import {Redirect} from '@docusaurus/router';

<Redirect to="/reference" />
````

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@SignorMercurio can I use this? Does it work?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No, the aim is not redirecting. Actually, reference-cli-bookkeeper.md (and the below) will not be shown on pulsar.apache.org. Rather, it'll be shown only in pulsar.apache.org/reference. Therefore, the frontmatter is no longer useful as it only works for Docusaurus.

Comment thread site2/docs/reference-cli-broker-tool.md Outdated
Comment on lines -1 to -5
---
id: reference-cli-broker-tool
title: Pulsar CLI tools - broker-tool
sidebar_label: "cli-broker-tool"
---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

title: Pulsar CLI tools - pulsar-daemon
sidebar_label: "cli-pulsar-daemon"
---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto

@Anonymitaet

Copy link
Copy Markdown
Member Author

Hi @tisonkun @SignorMercurio I‘ve updated based on your comments:

  • Remove reference-cli-bookkeeper.md and reference-cli-pulsar-daemon.md
  • Update URL

PTAL and feel free to approve, thanks.

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generally looks good. I suggest you leave the removing and moving reference cli docs task to @SignorMercurio so that he can simultaneously handle the process.

Otherwise, site build can fail and it takes more effort to recover these files and copy to the pulsar-site repo.

That is, do not remove:

  • reference-cli-bookkeeper.md
  • reference-cli-broker-tool.md
  • reference-cli-pulsar-daemon.md

in this pull request. I think @SignorMercurio will handle it later.

@Anonymitaet

Copy link
Copy Markdown
Member Author

Generally looks good. I suggest you leave the removing and moving reference cli docs task to @SignorMercurio so that he can simultaneously handle the process.

Otherwise, site build can fail and it takes more effort to recover these files and copy to the pulsar-site repo.

That is, do not remove:

  • reference-cli-bookkeeper.md
  • reference-cli-broker-tool.md
  • reference-cli-pulsar-daemon.md

in this pull request. I think @SignorMercurio will handle it later.

@tisonkun OK, I've updated this PR based on your comments, PTAL

@Anonymitaet

Copy link
Copy Markdown
Member Author

Ping @momo-jun to review

@Anonymitaet

Copy link
Copy Markdown
Member Author

ping @momo-jun

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

doc Your PR contains doc changes, no matter whether the changes are in markdown or code files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants