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

Jump to bottom

update the old content of chinese website #10876

Closed
wants to merge 7 commits into from
Closed

update the old content of chinese website #10876

wants to merge 7 commits into from

Conversation

yeya24
Copy link

@yeya24 yeya24 commented Nov 3, 2018

fix a typo in line 173 Kubernets -> Kubernetes

fix a typo Kubernets -> Kubernetes
19b32d0 
fix a typo in line 173 Kubernets -> Kubernetes
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 3, 2018
@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Nov 3, 2018
@k8sio-netlify-preview-bot
Copy link
Collaborator

Deploy preview for kubernetes-io-master-staging failed.

Built with commit 19b32d0

https://app.netlify.com/sites/kubernetes-io-master-staging/deploys/5bdcfc34b312745fd1344beb

@yeya24
Copy link
Author

yeya24 commented Nov 3, 2018

/assign @vishh

@k8sio-netlify-preview-bot
Copy link
Collaborator

k8sio-netlify-preview-bot commented Nov 3, 2018 •
edited

Deploy preview for kubernetes-io-master-staging ready!

Built with commit 1200ddc

https://deploy-preview-10876--kubernetes-io-master-staging.netlify.com

@idealhack
Copy link
Member

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 3, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: idealhack
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: vishh

If they are not already assigned, you can assign the PR to them by writing /assign @vishh in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@idealhack
Copy link
Member

@yeya24 Thanks for your contribution! You are updating an outdated page, it looks like haven't been updated for one year and a half at least! Would you like to update it with the English version?

@yeya24
Copy link
Author

yeya24 commented Nov 3, 2018

Thanks for your reply @idealhack ! I missed that it is an outdated page. So should I close this pr by myself?

@idealhack
Copy link
Member

@yeya24 No need to close this PR, what I mean is since you are fixing this page, we'd like to see you could keep it updated.

If you can do this shortly, you can just push it to this branch, and edit the PR description. Otherwise, you can open a new PR.

update the old version of the scheduling of gpu
6a7a0a4 
update the old version of the scheduling of gpu
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Nov 3, 2018
@yeya24
Copy link
Author

yeya24 commented Nov 3, 2018

@idealhack Hi, I have updated the old version. If there is some problems, please check it out and tell me.
Thanks!

@yeya24 yeya24 changed the title fix a typo Kubernets -> Kubernetes update the old content of chinese website Nov 5, 2018
@idealhack
Copy link
Member

🎉 thanks! will review it tomorrow

/cc

@idealhack
Copy link
Member

I'm not sure if we have some guidelines about spaces here, but I think this is a good one, @yeya24 could you update accordingly?

fix and update the page
37ab5c3 
fix and update the page
@yeya24
Copy link
Author

yeya24 commented Nov 6, 2018 •
edited

I'm not sure if we have some guidelines about spaces here, but I think this is a good one, @yeya24 could you update accordingly?

@idealhack Already update it. Please review it~

idealhack
idealhack reviewed Nov 7, 2018
View reviewed changes
Copy link
Member

@idealhack idealhack left a comment

Choose a reason for hiding this comment

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

This is awesome! Only a few nits.

/lgtm

content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2018
fix
bdbcb4c 
fix
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 7, 2018
@k8s-ci-robot
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@yeya24
Copy link
Author

yeya24 commented Nov 7, 2018

thank u so much for your patience! @idealhack

@Bradamant3 Bradamant3 added the language/zh Issues or PRs related to Chinese language label Nov 9, 2018
markthink
markthink reviewed Nov 18, 2018
View reviewed changes
content/zh/docs/tasks/manage-gpus/scheduling-gpus.md
{{% /capture %}}

{{% capture prerequisites %}}
Kubernetes 提供对分布在节点上的 AMD 和 NVIDIA GPU 进行管理的**实验**支持。对 NVIDIA GPU 的支持在 v1.6 中加入，已经经历了多次不向后兼容的迭代。而对 AMD GPU 的支持则在 v1.9 中通过 [device plugin](#deploying-amd-gpu-device-plugin) 加入。
Copy link

Choose a reason for hiding this comment

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

原始的英文怎么移掉了

Copy link
Member

Choose a reason for hiding this comment

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

什么意思？

Copy link

Choose a reason for hiding this comment

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

参考：https://github.com/k8smeetup/k8s-official-translation

Copy link
Member

Choose a reason for hiding this comment

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

看起来这篇中文本来就没有带英文，而且相当老。

附带英文的做法在一开始可行，但会造成需要维护两份英文文章，可以改为加链接的方式，review 的时候点过去看对应英文就好。

Copy link

Choose a reason for hiding this comment

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

之前的基准是 master 分支，往 master 分支提交确实会有这个问题，不过现在不是以 master 分支为基准..所以不存在此总是。还是按规范走，方便 reviewer 的人比对原文。

由于注释格式是一致的，脱敏移除英文也是非常容易的，退一看，不移除注释，浏览器渲染也不会把注释原文展示出来。

@idealhack
Copy link
Member

@yeya24 Once you’ve fixed the links above and squash the commits we are ready to merge this.

@idealhack idealhack added the tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. label Nov 18, 2018
@markthink
Copy link

@yeya24 不建议向 master 分支提交，目前社区讨论的结果是以 release-1.12 为基准进行更新，建议重新提交一个符合标准的 PR 至 release-1.12 分支。

@idealhack idealhack changed the base branch from master to release-1.12 November 20, 2018 05:08
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 20, 2018
@idealhack
Copy link
Member

idealhack commented Nov 20, 2018 •
edited

我修改了分支， @yeya24 可以在 release-1.12 HEAD 上 rebase

（看起来没有冲突就应该没问题）

@idealhack
Copy link
Member

idealhack commented Nov 20, 2018 •
edited

@markthink 目前翻译的 PR 还有很多都指向 master。我们应该在 website 仓库或者 community 仓库中添加文档，说明翻译应该使用的分支规范，并且在 PR 的默认描述中提示 PR 作者。

update: 原来已经有相关文档了，只是还没有中文版？

@idealhack
Copy link
Member

idealhack commented Nov 20, 2018 •
edited

@yeya24 如有问题欢迎在 slack 或 wechat 联系我，如果放弃这个 PR，我可以新开一个 PR 将你作为 co-author

（新 PR 已开）

@idealhack idealhack changed the base branch from release-1.12 to master November 22, 2018 11:05
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Nov 22, 2018
@idealhack idealhack mentioned this pull request Nov 22, 2018
Merged
@idealhack
Copy link
Member

Closing since #11182 has been merged.

/close

@k8s-ci-robot
Copy link
Contributor

@idealhack: Closed this PR.

In response to this:

Closing since #11182 has been merged.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@idealhack idealhack removed tide/merge-method-squash Denotes a PR that should be squashed by tide when it merges. labels Nov 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@idealhack idealhack idealhack approved these changes

@markthink markthink markthink left review comments

Assignees

@idealhack idealhack

@vishh vishh

Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/zh Issues or PRs related to Chinese language size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@yeya24 @k8sio-netlify-preview-bot @idealhack @k8s-ci-robot @markthink @Bradamant3 @vishh