Skip to content

Reorganize docs contrib guide#9510

Merged
k8s-ci-robot merged 9 commits intokubernetes:masterfrom
mdlinville:contribute-to-docs-reorg
Aug 6, 2018
Merged

Reorganize docs contrib guide#9510
k8s-ci-robot merged 9 commits intokubernetes:masterfrom
mdlinville:contribute-to-docs-reorg

Conversation

@mdlinville
Copy link
Copy Markdown
Contributor

@mdlinville mdlinville commented Jul 13, 2018

Preview at https://deploy-preview-9510--kubernetes-io-master-staging.netlify.com/docs/contribute/

This is a HUGE PR to review. I'm very sorry about that. It does the following major things:

  • Rewrite, reorganize, and consolidate all docs about contributing under a new Contribute section of the website, under /docs/contribute/
  • Improve and update docs about page templates, content organization, etc
  • Change the behavior of the "Edit this page" links (at bottom and top right) to go straight to the Github editor with no preliminary page in between
  • Change the text of the "Create an issue" button to "Report a problem". Include both the page URL and file name in the PR subject. I'd love to have left the subject blank to force the reporter to write a meaningful title, and include the path and file name in the issue body, but that overrides the Github issue template. Also remove reliance on Javascript to build the "Create an issue" link.
  • Lots of redirects added

There are still some whitespace errors (trailing, mostly) that I need to find and fix. But this is ready to review.

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 13, 2018
@k8sio-netlify-preview-bot
Copy link
Copy Markdown
Collaborator

k8sio-netlify-preview-bot commented Jul 13, 2018

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

Built with commit ae0c7a0

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

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jul 18, 2018
@mdlinville
Copy link
Copy Markdown
Contributor Author

/hold cancel

@mdlinville mdlinville changed the title WIP Reorganize docs contrib guide Reorganize docs contrib guide Jul 20, 2018
@k8s-ci-robot 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 Jul 20, 2018
@mdlinville
Copy link
Copy Markdown
Contributor Author

/assign @tallt0m
/assign @chenopis
/assign @jaredbhatti
/assign @zacharysarah
/assign @Bradamant3

@k8s-ci-robot
Copy link
Copy Markdown
Contributor

@MistyHacks: GitHub didn't allow me to assign the following users: tallt0m.

Note that only kubernetes members and repo collaborators can be assigned.
For more information please see the contributor guide

Details

In response to this:

/assign @tallt0m
/assign @chenopis
/assign @jaredbhatti
/assign @zacharysarah
/assign @Bradamant3

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.

use multiples of 10, to account for adding topics later. For instance, a topic
with weight `10` will come before one with weight `20`.

## Including code from another file
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a PR under review for this sub-topic. #9574

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@tengqm Can you let me know when your PR is merged so I can make sure those changes get into this branch? I'm sure your PR will be ready before mine has been completely reviewed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just merged it. Let me now rebase this PR and integrate that content into the new location.

@@ -1,4 +1,5 @@
<!-- Thanks for filing an issue! Before submitting, please fill in the following information. -->
<!-- See https://kubernetes.io/docs/contribute/start/ for guidance on writing an actionable issue description. -->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice! Very cool to have a helpful pointer here.

@@ -1,6 +1,7 @@
>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Please delete this note before submitting the pull request.
> For 1.12 Features: set Milestone to 1.12 and Base Branch to release-1.12
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have instructions somewhere that provides an example on how to set the base branch to release-1.12? For folks new to git that would be very helpful. Also do we have instructions somewhere on setting the milestone. People will ask how to do that as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

>^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Please delete this note before submitting the pull request.
> For 1.12 Features: set Milestone to 1.12 and Base Branch to release-1.12
> For help editing and submitting pull requests, see https://kubernetes.io/contribute/start/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Very helpful!

and whether you self-identify as a developer, an end user, or someone who just
can't stand seeing typos.

Looking for the [style guide](/docs/contribute/style/style-guide/)?
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of "Looking for the" Perhaps say, Before you start contributing we highly recommend you review our [style guide] . This will help to get your pull requests merged more quickly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's convered in https://deploy-preview-9510--kubernetes-io-master-staging.netlify.com/docs/contribute/start/#style-guidelines. The one here is just for people who expected to find the style guide right at the top of the contribute/ level, as a convenience.

- Participate in a Kubernetes release team as a docs representative
- Propose improvements to the style guide
- Propose improvements to docs tests
- Propose improvements to the Kubernetes website or other tooling
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should Approve and Merge PRs be on this list?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep

@bradtopol bradtopol self-assigned this Jul 22, 2018
currently work and why past decisions have been made before proposing sweeping
changes. The quickest way to get answers to questions about how the documentation
currently works is to ask in the #sig-docs Slack channel on
[kubernetes.slack.com](https://kubernetes.slack.com)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Great section!


### Approvers

Approvers have the ability to merge a PR.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I thought this was called maintainer

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Updated.


{{% capture overview %}}

When contributing new topics, apply one of the following templates to them.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So awesome to have this documented!!!!

| discussion | no |
| whatsnext | no |

The page's body will look like this (without any optional sections you don't
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So are any of the sections below optional or are they mandatory? Its unclear to me

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't understand. The "Required" column in the table should cover this, I thought.

- For `overview`, use a paragraph to set context for the entire topic.
- Add the `{{< toc >}}` shortcode to show an in-page table of contents.
- For `prerequisites`, use bullet lists when possible. Add additional
prerequisites below the ones included by default.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is is the < include "task-tutorial-prereqs.md> that are the default pre-requisites? This should be clarified

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added some clarification.

`/content/en/docs/tutorials` directory, with the following characteristics:

- In the page's YAML front-matter, set `content_template: templates/tutorial`.
- In the page's body, set the following `capture` variables:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps provide an example on how to set one of these variables

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think the inline examples show it pretty well, right?

## Write a blog post

Anyone can write a blog post and submit it for review. Blog posts should not be
commercial in nature and should consiste of content that will apply broadly to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fix: consiste


{{% capture prerequisites %}}

{{< include "task-tutorial-prereqs.md" >}} {{< version-check >}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is this basic version check necessary for every task/tutorial? I could understand using a version check if the instructions in the task pertain to a specific version, otherwise, it seems odd to remind the user to check their version without saying why.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was decided by the SIG when the templates were decided upon. Not my new change. :)

@zacharysarah
Copy link
Copy Markdown
Contributor

From Slack with @MistyHacks:

still awaiting some clarification on org member / sig review / sig approver, what the specific time requirements and time-commit expectations are of each, for the docs contributor guid

Here's a summary of the operative criteria. Hopefully I got everything--other maintainers, please chime in!

Role Docs prerequisites Responsibilities
Member - PRs must be to k/website Follow the code of conduct
Reviewer - As per member
- Regularly participate in weekly SIG meetings
- As per member
- Review PRs auto-assigned by blunderbuss within 1 week
Approver - As per reviewer
- Contribute regularly for at least 3 months
- Knowledgeable with SIG Docs workflows and tooling
- As per reviewer
- Respond to PR comments/requests for review from other maintainers within 1 week
- Serve periodically as the PR Wrangler for SIG Docs


When you are comfortable with all of the tasks discussed in this topic and you
want to engage with the Kubernetes docs team in deeper ways, read the
[internediate docs contribution guide](/docs/contribute/intermediate/).
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spelling nit: internediate

docs.

2. By default, the only filter that is applied is `open`, so you don't see
pull reuests that have already been closed or merged. It's a good idea to
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

spelling nit: reuests

@mdlinville
Copy link
Copy Markdown
Contributor Author

I rebased, fixed the nits pointed out by @kbhawkey , and updated the participating.md with more details and to include SIG Docs chair. Thanks everyone for your reviews. I think barring anything major that is wrong here, this is ready to go in and we can iterate.

/assign @chenopis

@mdlinville mdlinville dismissed zacharysarah’s stale review August 6, 2018 21:58

All of this feedback was addressed.

@mdlinville
Copy link
Copy Markdown
Contributor Author

Troubleshooting netlify failure.

@chenopis
Copy link
Copy Markdown
Contributor

chenopis commented Aug 6, 2018

Netlify build looks good:

3:39:04 PM: Build ready to start
3:39:05 PM: build-image version: 7c49b266ed8abd833dee6036ec0a4b9ee887658a
3:39:05 PM: buildbot version: f58df58ab0bceffdcb8d1e60fec992495a2a6751
3:39:05 PM: Fetching cached dependencies
3:39:06 PM: Starting to download cache of 314.6MB
3:39:07 PM: Finished downloading cache in 1.403494345s
3:39:07 PM: Starting to extract cache
3:39:13 PM: Finished extracting cache in 6.232225962s
3:39:13 PM: Finished fetching cache in 7.787273586s
3:39:13 PM: Starting to prepare the repo for build
3:39:14 PM: Preparing Git Reference pull/9510/head
3:39:20 PM: Found netlify.toml. Overriding site configuration
3:39:20 PM: Different build command detected, going to use the one specified in the toml file: 'hugo --enableGitInfo -b $DEPLOY_PRIME_URL' versus 'hugo' in the site
3:39:20 PM: Starting build script
3:39:20 PM: Installing dependencies
3:39:21 PM: Started restoring cached node version
3:39:23 PM: Finished restoring cached node version
3:39:24 PM: v8.11.3 is already installed.
3:39:25 PM: Now using node v8.11.3 (npm v5.6.0)
3:39:25 PM: Attempting ruby version 2.2.9, read from environment
3:39:26 PM: Using ruby version 2.2.9
3:39:26 PM: Using PHP version 5.6
3:39:26 PM: Installing Hugo 0.46
3:39:26 PM: Started restoring cached go cache
3:39:26 PM: Finished restoring cached go cache
3:39:26 PM: unset GOOS;
3:39:27 PM: unset GOARCH;
3:39:27 PM: export GOROOT='/opt/buildhome/.gimme/versions/go1.10.linux.amd64';
3:39:27 PM: export PATH="/opt/buildhome/.gimme/versions/go1.10.linux.amd64/bin:${PATH}";
3:39:27 PM: go version >&2;
3:39:27 PM: export GIMME_ENV='/opt/buildhome/.gimme/env/go1.10.linux.amd64.env';
3:39:27 PM: go version go1.10 linux/amd64
3:39:27 PM: Installing missing commands
3:39:27 PM: Verify run directory
3:39:27 PM: Executing user command: hugo --enableGitInfo -b $DEPLOY_PRIME_URL
3:39:29 PM: Building sites …
3:39:37 PM:

3:39:37 PM: | EN | CN
3:39:37 PM: +------------------+------+-----+
3:39:37 PM: Pages | 1339 | 163
3:39:37 PM: Paginator pages | 232 | 0
3:39:37 PM: Non-page files | 405 | 267
3:39:37 PM: Static files | 803 | 803
3:39:37 PM: Processed images | 0 | 0
3:39:37 PM: Aliases | 3 | 1
3:39:37 PM: Sitemaps | 2 | 1
3:39:37 PM: Cleaned | 0 | 0
3:39:37 PM: Total in 8030 ms
3:39:37 PM: Caching artifacts
3:39:37 PM: Started saving pip cache
3:39:37 PM: Finished saving pip cache
3:39:37 PM: Started saving emacs cask dependencies
3:39:37 PM: Finished saving emacs cask dependencies
3:39:37 PM: Started saving maven dependencies
3:39:37 PM: Finished saving maven dependencies
3:39:37 PM: Started saving boot dependencies
3:39:37 PM: Finished saving boot dependencies
3:39:37 PM: Started saving go dependencies
3:39:37 PM: Finished saving go dependencies
3:39:37 PM: Build script success
3:39:37 PM: Starting to deploy site from 'public'
3:40:32 PM: Starting post processing
3:41:07 PM: Finished processing build request in 2m1.88368775s
3:43:02 PM: Post processing done
3:43:02 PM: Site is live

It was being interpreted as a Hugo shortcode.
@mdlinville
Copy link
Copy Markdown
Contributor Author

Confirmed the last commit fixed the bug that was breaking Netlify and also adds a related test to the smoke tests page. This is ready to go. 🍾

@chenopis
Copy link
Copy Markdown
Contributor

chenopis commented Aug 6, 2018

/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 Aug 6, 2018
@k8s-ci-robot
Copy link
Copy Markdown
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: chenopis

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

The pull request process is described here

Details 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 Aug 6, 2018
@k8s-ci-robot k8s-ci-robot merged commit 5ae0d0d into kubernetes:master Aug 6, 2018
@mdlinville mdlinville deleted the contribute-to-docs-reorg branch August 6, 2018 23:17
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants