Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing by celestehorgan · Pull Request #19109 · kubernetes/website

celestehorgan · 2020-02-13T11:41:44Z

Closes #19108. See also: #18878

This PR replaces the Start Contributing and Intermediate pages into topic/task-based subject areas. It also aims to reduce duplication of content that occurred between these pages.

Some notes:

I originally intended for this PR to also address the Advanced page, but we're already at +1100 lines and -1500 lines, and I don't think it's a good idea to make this PR much bigger, for the sake of potential reviewers.
This PR does not address broken links! It would be too messy. That will come in a seperate PR. Before I open that PR, however, I want to give people a day or two to take a look at this PR, to make sure there are no recommendations for major structural changes which could cause icky merge conflicts.
This is a substantial PR. If you have limited time to review, I suggest taking a look at the preview as a whole and then picking 1-2 pages to review in depth? However, the entire PR does need review at some point :)

Thanks in advance, everyone!

celestehorgan · 2020-02-13T11:43:11Z

/label do-not-merge/work-in-progress

(Edit: it appears I cannot do this myself. I'd love if someone could mark this as a WIP! Thank you!!)

k8s-ci-robot · 2020-02-13T11:43:12Z

@celestehorgan: The label(s) /label do-not-merge/work-in-progress cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other

Details

In response to this:

/label do-not-merge/work-in-progress

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.

k8s-ci-robot · 2020-02-13T11:43:59Z

@celestehorgan: The label(s) /label do-not-merge/work-in-progress cannot be applied. These labels are supported: api-review, community/discussion, community/maintenance, community/question, cuj/build-train-deploy, cuj/multi-user, platform/aws, platform/azure, platform/gcp, platform/minikube, platform/other

Details

In response to this:

/label do-not-merge/work-in-progress

(Edit: it appears I cannot do this myself. I'd love if someone could mark this as a WIP! Thank you!!)

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.

netlify · 2020-02-13T11:53:03Z

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

Built with commit e5d7f9f

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

Signed-off-by: Celeste Horgan <celeste@cncf.io>

celestehorgan · 2020-02-20T08:45:37Z

/remove-do-not-merge work-in-progress

content/en/docs/contribute/_index.md

content/en/docs/contribute/style/style-guide.md

content/en/docs/contribute/review/reviewing.md

content/en/docs/contribute/_index.md

remyleone · 2020-02-20T13:42:03Z

Congrats @celestehorgan this is impressive work!

kbhawkey · 2020-02-21T15:05:22Z

content/en/docs/contribute/advanced.md

 slug: advanced
 content_template: templates/concept
-weight: 30
+weight: 98


nit: You could created a separate PR for the extra space changes.
nit: I think it is suggested to use whole number intervals for the weight value (80, 90 ...).
nit: I'd like to see Advanced contributing closer to the middle of the list

Fix links to pages that are no longer available.

content/en/docs/contribute/localization.md

content/en/docs/contribute/_index.md

kbhawkey · 2020-02-21T15:13:20Z

@celestehorgan ,
As there is a lot to review (not recommended), would you provide the page preview links to the individual pages? I am in favor of addressing the links with the changed content.

k8s-ci-robot · 2020-02-27T10:25:09Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
To complete the pull request process, please assign sftim
You can assign the PR to them by writing /assign @sftim in a comment when ready.

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

Details

Needs approval from an approver in each of these files:

content/en/OWNERS

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

Signed-off-by: Celeste Horgan <celeste@cncf.io>

celestehorgan · 2020-03-02T16:30:59Z

@zacharysarah @kbhawkey @sftim @remyleone Thank you all for the extensive review on this! I've hopefully resolved all the issues, except for a few that I have open questions about. I'm going to start work on a separate PR that fixes links now :)

celestehorgan · 2020-03-06T12:57:33Z

Due to some Git branching/committing errors on my part(1), it looks as if I'm going to have to drop commits for link fixing into this PR as well :/

Since that will involve touching even more files, I want to make sure that everyone's more or less done with comments around content.

To my knowledge, the only thing I haven't addressed is @kbhawkey's general comment about removing the "working locally" section, which I'll address in a follow-up comment to this one.

If no one has any objections, I'll drop in those link-related commits on Monday.

(1) The commits for this PR were made to the master branch of my fork. I goofed.

celestehorgan · 2020-03-06T13:00:40Z

@kbhawkey

In response to:

The biggest piece that is missing for me (and really the only section I used when first contributing) is the content found in the existing Intermediate page, https://kubernetes.io/docs/contribute/intermediate/. Are you missing a page, working-locally?

Content about working locally from a fork lives here: https://deploy-preview-19109--kubernetes-io-master-staging.netlify.com/docs/contribute/new-content/new-content/#large-changes.

However, I think it brings up a good point – Small changes and Large changes seemed like transparent titles at the time and probably aren't cutting it. Do you think renaming these two sections Opening a PR using GitHub and Opening a PR using a fork would clarify this for you, or was there content specific to https://kubernetes.io/docs/contribute/intermediate/#work-from-a-local-clone that you think needs to be brought in?

kbhawkey · 2020-03-06T14:43:34Z

@kbhawkey

In response to:

The biggest piece that is missing for me (and really the only section I used when first contributing) is the content found in the existing Intermediate page, https://kubernetes.io/docs/contribute/intermediate/. Are you missing a page, working-locally?

Content about working locally from a fork lives here: https://deploy-preview-19109--kubernetes-io-master-staging.netlify.com/docs/contribute/new-content/new-content/#large-changes.

However, I think it brings up a good point – Small changes and Large changes seemed like transparent titles at the time and probably aren't cutting it. Do you think renaming these two sections Opening a PR using GitHub and Opening a PR using a fork would clarify this for you, or was there content specific to https://kubernetes.io/docs/contribute/intermediate/#work-from-a-local-clone that you think needs to be brought in?

Hello @celestehorgan .
I quickly reread the small changes/large changes page. I like the simplicity of the page, but find that there are a number of details that are missing for successfully committing a docs PR.
I wonder what percentage of authors use the GitHub UI to process a PR vs fork+clone?
The Large changes section gets off to a good start by setting up a local clone, but does not explicitly provide the commands to push a PR.

If you are depending upon the k8s community PR workflow instructions to take the author through the process, then perhaps this information should be put into a note (highlight what to do).
First time contributors seem to pause when reviewers ask for changes to the PR.
How to do this? How to keep your clone in sync? How to make changes iteratively to the same PR?
When to squash/when not to squash?

celestehorgan · 2020-03-06T15:54:06Z

@kbhawkey

All good points, thank you for clarifying. Instead of dropping link corrections now, let add content to address your points, and after that I'll cherrypick the link corrections to this branch.

content/en/docs/contribute/_index.md

content/en/docs/contribute/suggesting-improvements.md

content/en/docs/contribute/new-content/overview.md

kbhawkey · 2020-03-09T00:27:34Z

content/en/docs/contribute/new-content/contributing-content.md

    or just close the terminal window.
 {{% /tab %}}
-{{% tab name="Hugo locally" %}}
+{{% tab name="Hugo on the command line" %}}


nit: Should the heading emphasize the repo name,

### Fork the kubernetes/website repository

As in: change Line 85's title to "Fork the kubernetes/website repository?

Yes. This is a nit. Should a heading emphasize the repository name?

@kbhawkey Apparently no. I'll make this change and do a sweep through the rest of the PR.

content/en/docs/contribute/review/reviewing-prs.md

content/en/docs/contribute/review/for-approvers.md

k8s-ci-robot · 2020-03-09T13:19:22Z

@celestehorgan: PR needs rebase.

Details

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.

celestehorgan · 2020-03-09T15:40:36Z

To rebase this and solve the merge conflict, I'd prefer to close this PR and re-open it. As I mentioned previously, I committed these directly to my fork's master – I want to redo this as a branch, so I don't risk bringing in stray commits that don't belong here when I do the rebase/conflict resolution. This might make it cleaner for me to add link correction commits, too :)

WDYT? Any requests for when I re-open this PR?

sftim · 2020-03-09T16:21:02Z

@celestehorgan it's fine if you want to do that!

You could:

git checkout celestehorgan/master # if your fork's remote is called "celestehorgan"
git checkout -b new-branch-name
git fetch kubernetes master
git rebase -i kubernetes/master # can be fiddly if you haven't done it before
git push -u celestehorgan new-branch-name:new-branch-name

then open a new pull request that mentions this one
Finally, close this pull request.

Does that make sense?

kbhawkey · 2020-03-09T16:41:03Z

@celestehorgan it's fine if you want to do that!

You could:
git checkout celestehorgan/master # if your fork's remote is called "celestehorgan"
git checkout -b new-branch-name
git fetch kubernetes master
git rebase -i kubernetes/master # can be fiddly if you haven't done it before
git push -u celestehorgan new-branch-name:new-branch-name
then open a new pull request that mentions this one
Finally, close this pull request.

Does that make sense?

👍

celestehorgan · 2020-03-09T18:31:32Z

Closing to re-open in a less messy way.

k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Feb 13, 2020

celestehorgan changed the title ~~Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing~~ [WIPRestructure/De-duplicate information in Start/Intermediate/Advanced Contributing Feb 13, 2020

celestehorgan changed the title ~~[WIPRestructure/De-duplicate information in Start/Intermediate/Advanced Contributing~~ [WIP] Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing Feb 13, 2020

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. language/en Issues or PRs related to English language sig/docs Categorizes an issue or PR as relevant to SIG Docs. labels Feb 13, 2020

k8s-ci-robot requested review from daminisatya and jaredbhatti February 13, 2020 11:42

Restructure to reduce duplication

7dd2c93

Signed-off-by: Celeste Horgan <celeste@cncf.io>

celestehorgan force-pushed the master branch from cc3da4c to 7dd2c93 Compare February 19, 2020 16:41

celestehorgan changed the title ~~[WIP] Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing~~ Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing Feb 20, 2020

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Feb 20, 2020

celestehorgan mentioned this pull request Feb 20, 2020

Restructure/De-duplicate information in Start/Intermediate/Advanced Contributing #19108

Closed

sftim reviewed Feb 20, 2020

View reviewed changes

content/en/docs/contribute/_index.md Show resolved Hide resolved