Skip to content

Conversation

@gxlct008
Copy link
Contributor

@gxlct008 gxlct008 commented Apr 2, 2021

What this PR does / why we need it:

improve Getting Started in Chinese

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

@juzhiyuan juzhiyuan requested review from LiteSun and spacewander April 2, 2021 13:01
@juzhiyuan
Copy link
Member

Thanks for your contribution!

@juzhiyuan
Copy link
Member

BTW, I see there have some lint errors:

image

Would you like to fix them first?

@gxlct008
Copy link
Contributor Author

gxlct008 commented Apr 3, 2021

@juzhiyuan
I have fixed all the problems with the Markdown format.

Copy link
Member

@juzhiyuan juzhiyuan left a comment

Choose a reason for hiding this comment

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

Hi, @gxlct008 Thanks for your contribution!

BTW, I just commented on some changes that could make our doc easier to read :) You could simply click the commit button.

@juzhiyuan
Copy link
Member

GitHub commits suggestion is too hard to use.. maybe it doesn't work for many suggestions case

@juzhiyuan juzhiyuan requested a review from moonming April 5, 2021 01:41

```bash
curl http://127.0.0.1:9080/apisix/admin/routes/5 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
$ curl http://127.0.0.1:9080/apisix/admin/routes/5 -H 'X-API-KEY: edd1c9f034335f136f87ad84b625c8f1' -X PUT -d '
Copy link
Member

Choose a reason for hiding this comment

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

Actually I think it would be a good idea to remove all the $, so users could directly use the copy button on the website to get the 'runnable' command. cc @juzhiyuan

Another reason could be, the dollar sign is often used when trying to separate commands and outputs. Since in this doc, we only list the command, so there seems no need to use $

Copy link
Member

Choose a reason for hiding this comment

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

it's reasonable though I would prefer adding $ 😂 what do you think? @spacewander

Copy link
Member

Choose a reason for hiding this comment

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

Found some examples:

So it seems the copy button could be customized

Copy link
Member

Choose a reason for hiding this comment

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

😂 wow, that button could omit $

Copy link
Member

Choose a reason for hiding this comment

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

We can omit the $. $ is not part of the bash script.

@spacewander spacewander changed the title doc: update the Getting Started guide in Chinese docs: update the Getting Started guide in Chinese Apr 7, 2021

- 本指南使用 docker 和 docker compose 来安装 Apache APISIX。 但是, 如果您已经以其他方式安装了 Apache APISIX ,您只需跳到 [第二步](getting-started.md#第二步:-在-APISIX-中设置路由)
- Curl:指南使用 Curl 命令进行 API 测试,但是您也可以使用您选择的任何其他工具( 例如 Postman )。
> 如果您已经安装了 Apache APISIX,请直接阅读[第二步](#step-2-create-a-route)
Copy link
Member

Choose a reason for hiding this comment

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

We should change the anchor?

```

- Upstream 信息
这条路由配置是指:当入站请求满足下述规则时,请求将被转发到 `httpbin.org:80` 上游
Copy link
Member

Choose a reason for hiding this comment

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

少翻译了“全部”

"uri": "/samplePrefix/get",
"plugins": {
"proxy-rewrite": {
"scheme": "https",
Copy link
Member

Choose a reason for hiding this comment

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

Should remove the scheme

Co-authored-by: 琚致远 <juzhiyuan@apache.org>
@spacewander spacewander merged commit 3549829 into apache:master Apr 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants