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.

Sign up for GitHub

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

Revise L10n contribution guidelines #13850

Merged
merged 1 commit into from
Apr 19, 2019
Merged

Revise L10n contribution guidelines #13850

merged 1 commit into from
Apr 19, 2019

Conversation

zacharysarah
Copy link
Contributor

@zacharysarah zacharysarah commented Apr 17, 2019 •
edited
Loading

Summary

This PR updates contribution guidelines for localization. Revisions are based on the experience of localizations so far and to reflect the workflow in real practice.

Reviewers

@rlenferink @remyleone 👋 As the contributors with the most domain knowledge about adding a new localization, your feedback is invaluable.

@gochist @claudiajkang 👋 Does the section on branching strategy still reflect best practice?

/assign @rlenferink @remyleone
/cc @kubernetes/sig-docs-l10n-admins @Bradamant3 @jaredbhatti

Easy preview link

https://deploy-preview-13850--kubernetes-io-master-staging.netlify.com/docs/contribute/localization/

@k8s-ci-robot k8s-ci-robot requested review from Bradamant3, jaredbhatti and a team April 17, 2019 00:20
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 17, 2019
@zacharysarah zacharysarah force-pushed the update-l10n-guide branch 5 times, most recently from 0194d8b to 87be878 Compare April 17, 2019 00:28
@netlify
Copy link

netlify bot commented Apr 17, 2019

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

Built with commit a9ee932

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

@netlify
Copy link

netlify bot commented Apr 17, 2019 •
edited
Loading

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

Built with commit f82d796

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

@DanyC97
Copy link
Contributor

DanyC97 commented Apr 17, 2019

it looks great @zacharysarah , left minor nits

@gochist
Copy link
Member

gochist commented Apr 17, 2019 •
edited
Loading

@zacharysarah Thank you for asking about this. I tried to explain how sig-docs-ko-owners team works in real by another PR #13859 . (EDITED)

@gochist gochist mentioned this pull request Apr 17, 2019
Closed
rlenferink
rlenferink reviewed Apr 17, 2019
View reviewed changes
Copy link
Member

@rlenferink rlenferink left a comment

Choose a reason for hiding this comment

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

Looks good @zacharysarah ! A few minor things, but in general, good job ! 😃

content/en/docs/contribute/localization.md Outdated Show resolved Hide resolved
content/en/docs/contribute/localization.md Outdated Show resolved Hide resolved
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 17, 2019
@zacharysarah zacharysarah force-pushed the update-l10n-guide branch 2 times, most recently from 207151e to 62f834f Compare April 17, 2019 22:05
@remyleone
Copy link
Contributor

/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 Apr 18, 2019
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 18, 2019
gochist
gochist reviewed Apr 19, 2019
View reviewed changes
content/en/docs/contribute/localization.md Outdated
@@ -183,22 +198,28 @@ To collaborate on a development branch:

3. Approvers review and merge feature branches into the development branch.

4. Periodically, an approver merges the development branch to its source branch.
4. Periodically, an approver merges the development branch to its source branch by opening and approving a new pull request. Be sure to [squash the commits](https://help.github.com/en/articles/creating-a-commit-with-multiple-authors) before approving the pull request.
Copy link
Member

Choose a reason for hiding this comment

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

@zacharysarah I think the link is helpful but it explains about creating co-authored commit while the readers may expect it contains how to squash the commits.

Revise L10n contribution guidelines
f82d796 
Plain language rocks

Remove an orphaned line

Remove note

Present tense rocks

Add repository

Feedback, round 1

Add team descriptions

Add Slack link

Feedback, part 2

Feedback from gochist

Remove link
@DanyC97
Copy link
Contributor

DanyC97 commented Apr 19, 2019

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 19, 2019
@Bradamant3
Copy link
Contributor

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Bradamant3, remyleone

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 19, 2019
@k8s-ci-robot k8s-ci-robot merged commit 251eb0d into kubernetes:master Apr 19, 2019
@zacharysarah zacharysarah deleted the update-l10n-guide branch April 19, 2019 22:17
nowjean pushed a commit to nowjean/website that referenced this pull request Jun 13, 2019
@zacharysarah @nowjean
Revise L10n contribution guidelines (kubernetes#13850)
7c15d0b 
Plain language rocks

Remove an orphaned line

Remove note

Present tense rocks

Add repository

Feedback, round 1

Add team descriptions

Add Slack link

Feedback, part 2

Feedback from gochist

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

@rlenferink rlenferink rlenferink left review comments

@DanyC97 DanyC97 DanyC97 left review comments

@kbhawkey kbhawkey kbhawkey left review comments

@remyleone remyleone remyleone left review comments

@gochist gochist gochist left review comments

@Bradamant3 Bradamant3 Awaiting requested review from Bradamant3

@jaredbhatti jaredbhatti Awaiting requested review from jaredbhatti

Assignees

@remyleone remyleone

@DanyC97 DanyC97

@rlenferink rlenferink

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. language/en Issues or PRs related to English language lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/docs Categorizes an issue or PR as relevant to SIG Docs. 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.
8 participants
@zacharysarah @DanyC97 @gochist @remyleone @Bradamant3 @k8s-ci-robot @kbhawkey @rlenferink