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

Init Polish localization #18419

Merged
merged 9 commits into from
Jan 12, 2020
Merged

Conversation

mfilocha
Copy link
Contributor

@mfilocha mfilocha commented Jan 2, 2020

An initial config for Polish localization.

Kubernetes slogan translated by Karol Pucynski
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Jan 2, 2020
@k8s-ci-robot k8s-ci-robot added the do-not-merge/invalid-owners-file Indicates that a PR should not merge because it has an invalid OWNERS file in it. label Jan 2, 2020
@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

Am I right that this is a work-in-progress?

Copy link
Contributor

@sftim sftim left a comment

Choose a reason for hiding this comment

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

I think this PR should either:

  • target a new branch, not master
  • add Polski (pl) to disableLanguages

At the moment this pull request does not look OK to merge.

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

@mfilocha would you like help with, for example, getting a new branch made for the Polish work?

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 2, 2020

@sftim - @jimangel and @rlenferink asked me to reopen my previous PR against the master branch: #18254 (comment)

@rlenferink
Copy link
Member

rlenferink commented Jan 2, 2020

@sftim, we want to have this merged to master (our latest) branch as described by Jim as well. Why do you want to add pl to the disableLanguages? This PR adds all the initial configs and it will be a lot easier to translate and review when changes will be deployed for every PR.

As I understand @mfilocha processed all feedback from the previous pull request and then created this one. The only thing left to do is for @nvtkaszpir to request org membership (because OWNERS files are checked automatically) (@remyleone and me offered to sponsor).

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 2, 2020

@remyleone

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

This pull request does not yet add the minimum required content for a new localization. For that reason, it's not ready for approval.

According to the contribution guide, this new localization needs:

  • A translation of the CNCF code of conduct into Polish
  • A home page with all heading and subheading URLs
  • A setup section - https://kubernetes.io/pl/docs/setup/ - with heading and subheading URLs
  • The Kubernetes basics tutorial
  • The Hello Minikube tutorial
  • All site strings in a new localized TOML file

That's quite some work. So I'm recommending starting a new branch and working on making a series of changes to the new branch, until it is ready to merge into master.

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

@kubernetes/sig-docs-l10n-admins can you advise on whether I've [mis]understood?

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 2, 2020

I'll follow whatever is the recommended way.
My biggest concern is that the PR containing all translated minimum will be of XXL size. And who will review this PR for correctness/style of translation?

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

@mfilocha you're talking about the merge from the Polish localization dev branch into master? Because that XXL merge will consist only of already-approved changes, it won't need code review, just signoff.

For an example, see commit 487505d from PR #16551. I'm assuming that this single commit is the result of squashing a branch that includes many PRs (I haven't actually done one of these; that's what it looks like to me).

@remyleone remyleone self-assigned this Jan 2, 2020
@remyleone
Copy link
Contributor

remyleone commented Jan 2, 2020

I think targetting master is the way to go to start a localization and can be proven by the following successfully merged init localization: #12548 #16965

@remyleone
Copy link
Contributor

@mfilocha got already a lot of content translated in Polish in his own fork ( https://github.com/mfilocha/website/tree/dev-1.16-pl.1 ).
My take would be to merge this initial PR as quickly as possible to make @mfilocha and the rest of the Polish translation team as autonomous as possible. Having this PR merged will make them able to quickly merge a lot of already made content that will be beneficial to the Polish-speaking users.

Slowing it down with extra requirements (that were not enforced for instance in the past of the Vietnamese translation) is not justified in this case IMHO.

@nvtkaszpir let me know when you open your org membership request, I will give my sponsoring to it.

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

Also see #17388

I agree: the current guidelines aren't clear, but there's a bug open to fix that.

@sftim
Copy link
Contributor

sftim commented Jan 2, 2020

@remyleone can I follow up on #18419 (comment) ?

#12548 looks like an example of a valid new localization, whereas it looks to me as if PR #16965 was merged too early (it should have waited on the changes that came in #17655). The Vietnamese localization team are doing a great job though, they've landed lots of good PRs since that first one.

@mfilocha I do not think that https://deploy-preview-18419--kubernetes-io-master-staging.netlify.com/pl/ looks good to merge; to me, this pull request looks like a strong work-in-progress. You can /retitle [WIP] Init Polish localization if you agree that there's more to do.

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 2, 2020

I wonder if dropping config.toml from this PR could help to avoid the scenario with the empty page as pointed by @sftim?
If I'm right this may allow us to follow the way set out by @remyleone in #18419 (comment).

@remyleone
Copy link
Contributor

@sftim I disagree and think that a team should be able to show results as fast as possible and I'm not in favor of enforcing an arbitrary number of PR before they can have the gratification to see their work live. Wikipedia is a clear demonstration of this principle. Plus as you trust/empower a team as early as possible they have all agency to show other potential contributors that they can to quickly have an impact. All actions that delay the gratification of seeing your work in production should be removed in IMHO

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 3, 2020

PR to create a label for Polish language kubernetes/test-infra#15684 is on hold waiting for this PR to be merged.

@sftim
Copy link
Contributor

sftim commented Jan 3, 2020

To make https://deploy-preview-18419--kubernetes-io-master-staging.netlify.com/pl/ look right you need at least an _index.html page and to localize all the strings it uses.

The way I see it, the minimum guidelines do indeed cover what's necessary to get a sensible-looking preview rendering OK for a new localization.

There are 2 “essential” pages in the minimum requirements; one is Hello Minikube and the other is the Kubernetes basics tutorial. I can see why those are included, they are really useful for people new to Kubernetes. It's OK to launch a localization once those 2 pages are ready, even before the remaining hundreds of pages are ready to publish.

@mfilocha
Copy link
Contributor Author

mfilocha commented Jan 3, 2020

@sftim All required translated content is ready at my branch: master...mfilocha:dev-1.17-pl.1-master - I'm going to split it into smaller PRs for review.
But this should happen on a separate branch in the upstream repository, as soon as the localization team is established, I guess.

@sftim
Copy link
Contributor

sftim commented Jan 3, 2020

So it sounds to me that if an admin creates a new branch dev-1.17-pl.1 in this repo, @mfilocha you can send in pull requests that target that branch and that you're all ready to do that?

(and then those commits can merge into master in a single PR that does preview OK)

Have I got that right?

@mfilocha
Copy link
Contributor Author

/assign @zacharysarah

@sftim
Copy link
Contributor

sftim commented Jan 11, 2020

@nvtkaszpir can you add /lgtm to this PR if you're happy with it? You're listed as a future reviewer.

@k8s-ci-robot
Copy link
Contributor

@kpucynski: changing LGTM is restricted to collaborators

In response to this:

/lgtm

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.

@nvtkaszpir
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 12, 2020
@rlenferink
Copy link
Member

2 times /lgtm by pl-reviewers. I think this is good to be merged @sftim 🎉

@jimangel
Copy link
Member

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jimangel

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 Jan 12, 2020
@k8s-ci-robot k8s-ci-robot merged commit 8996e30 into kubernetes:dev-1.17-pl.1 Jan 12, 2020
@zacharysarah
Copy link
Contributor

Congratulations! 🎉

mfilocha added a commit to mfilocha/website that referenced this pull request Jan 13, 2020
Initial commit for Polish localization

* Translation style reviewed and improved by:
  Michał Sochoń <[email protected]>
  Karol Pucyński <[email protected]>
  with contribution from Wojciech Kocjan <[email protected]>

* Kubernetes slogan translated by Karol Pucyński
* Corectness of non-language-specific content reviewed and improved by:
  Tim Bannister <[email protected]>
mfilocha added a commit to mfilocha/website that referenced this pull request Jan 13, 2020
Initial commit for Polish localization

* Translation style reviewed and improved by:
  Michał Sochoń <[email protected]>
  Karol Pucyński <[email protected]>
  with contribution from Wojciech Kocjan <[email protected]>

* Kubernetes slogan translated by Karol Pucyński
* Corectness of non-language-specific content reviewed and improved by:
  Tim Bannister <[email protected]>
mfilocha added a commit to mfilocha/website that referenced this pull request Jan 13, 2020
Initial commit for Polish localization

* Translation style reviewed and improved by:
  Michał Sochoń <[email protected]>
  Karol Pucyński <[email protected]>
  with contribution from Wojciech Kocjan <[email protected]>

* Kubernetes slogan translated by Karol Pucyński
* Corectness of non-language-specific content reviewed and improved by:
  Tim Bannister <[email protected]>
k8s-ci-robot pushed a commit that referenced this pull request Jan 14, 2020
Initial commit for Polish localization

* Translation style reviewed and improved by:
  Michał Sochoń <[email protected]>
  Karol Pucyński <[email protected]>
  with contribution from Wojciech Kocjan <[email protected]>

* Kubernetes slogan translated by Karol Pucyński
* Corectness of non-language-specific content reviewed and improved by:
  Tim Bannister <[email protected]>
@sftim
Copy link
Contributor

sftim commented Jan 14, 2020

/language pl

@k8s-ci-robot k8s-ci-robot added the language/pl Issues or PRs related to Polish language label Jan 14, 2020
@mfilocha mfilocha deleted the init-pl-l10n branch January 15, 2020 10:09
wawa0210 pushed a commit to wawa0210/website that referenced this pull request Mar 2, 2020
Initial commit for Polish localization

* Translation style reviewed and improved by:
  Michał Sochoń <[email protected]>
  Karol Pucyński <[email protected]>
  with contribution from Wojciech Kocjan <[email protected]>

* Kubernetes slogan translated by Karol Pucyński
* Corectness of non-language-specific content reviewed and improved by:
  Tim Bannister <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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/pl Issues or PRs related to Polish 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
Development

Successfully merging this pull request may close these issues.

9 participants