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

Add `docs` section to pull request template #79361

Merged
merged 1 commit into from Jul 15, 2019

Conversation

@saschagrunert
Copy link
Member

commented Jun 25, 2019

What type of PR is this?

/kind documentation

What this PR does / why we need it:

We are able to add further documentation to the release notes by parsing
an additional block at pull requests, for example:

- Description https://mylink.com

This commit adds this block to the pull request template.

Which issue(s) this PR fixes:

Relates to kubernetes/sig-release#667, kubernetes/release#741

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE
@saschagrunert

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2019

/sig release

**Additional documentation, like links to enhancement proposals (KEP)**:
Please use the following format for linking documentation: `- [DESCRIPTION] [LINK]`
```docs
- Relevant KEP https://github.com/kubernetes/enhancements/...

This comment has been minimized.

Copy link
@nikhita

nikhita Jun 25, 2019

Member

Haven't actually seen the code diff in k/release yet, but would it be useful to have a delimiter between [DESCRIPTION] and [LINK] to come to know when the description ends and when the link begins? Kinda rare, but I'm wondering what would happen if a description has a link... 🤔

Also, can we have multiple links for one description? I'm asking this from a completely noob perspective :) .. for example, can I do a Related doc PRs <link> <link> <link>?

This comment has been minimized.

Copy link
@saschagrunert

saschagrunert Jun 25, 2019

Author Member

I created a follow up PR for stripping away separators like - and : here: kubernetes/release#748 This should solve the delimiter problem 😇

Hm, multiple links to a single entry are currently not supported, for that we would have to create a new line, which equals a new entry. What works is omiting the description, like:

- https://google.com
@nikhita

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@nikhita

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@@ -42,3 +42,10 @@ Enter your extended release note in the block below. If the PR requires addition
```release-note
```
<!--
**Additional documentation, like links to enhancement proposals (KEP)**:

This comment has been minimized.

Copy link
@cblecker

cblecker Jun 25, 2019

Member

Should this line be a part of the comment too?

This comment has been minimized.

Copy link
@saschagrunert

saschagrunert Jun 26, 2019

Author Member

I think so, adding further documentation is not an optional step. :)

This comment has been minimized.

Copy link
@cblecker

cblecker Jul 2, 2019

Member

The problem is, because it's inside the comment, nobody will see it. The comment is only for instructions.

This comment has been minimized.

Copy link
@saschagrunert

saschagrunert Jul 3, 2019

Author Member

Alright, changed it that we only comment the actual instructions.

@saschagrunert saschagrunert force-pushed the openSUSE:docs-pr branch from 2026529 to c543811 Jul 3, 2019

@k8s-ci-robot k8s-ci-robot added size/S and removed size/XS labels Jul 3, 2019

@saschagrunert saschagrunert force-pushed the openSUSE:docs-pr branch 2 times, most recently from a6e16f2 to 8bf1b6f Jul 3, 2019

@k8s-ci-robot k8s-ci-robot added size/XS and removed size/S labels Jul 3, 2019

@saschagrunert saschagrunert force-pushed the openSUSE:docs-pr branch from 8bf1b6f to 7a9f0a8 Jul 3, 2019

@saschagrunert

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

/test pull-kubernetes-e2e-gce-100-performance

@saschagrunert

This comment has been minimized.

Copy link
Member Author

commented Jul 4, 2019

/retest

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 4, 2019

Great addition, @saschagrunert!
/lgtm
/approve

As an aside, I wonder if we can/should force docs on a PR e.g. kind/feature must have docs.
Well I'm sure it's possible, I'm just wondering if it's feasible...

@cblecker

This comment has been minimized.

Copy link
Member

commented Jul 4, 2019

If there are no docs they can either remove the section of leave it blank. For now the release notes tool is the only consumer of this section, so it is bound to the notes. But later on we could consume this documentation section by other tools as well, so I would keep the flexibility there. :) WDYT?

This guidance is missing from the instructions. We need to advise folks what to do in these use cases. We should explicitly state that it's only needed if you provide a release note, otherwise, leave it blank.

@saschagrunert saschagrunert force-pushed the openSUSE:docs-pr branch from ca59502 to 6f75b61 Jul 5, 2019

@k8s-ci-robot k8s-ci-robot removed the lgtm label Jul 5, 2019

@saschagrunert

This comment has been minimized.

Copy link
Member Author

commented Jul 5, 2019

If there are no docs they can either remove the section of leave it blank. For now the release notes tool is the only consumer of this section, so it is bound to the notes. But later on we could consume this documentation section by other tools as well, so I would keep the flexibility there. :) WDYT?

This guidance is missing from the instructions. We need to advise folks what to do in these use cases. We should explicitly state that it's only needed if you provide a release note, otherwise, leave it blank.

I added a notice about leaving it blank if there are no release notes provided.

@justaugustus
Copy link
Member

left a comment

@saschagrunert -- Coming back to this with a few nits that I've made as code suggestions.

Show resolved Hide resolved .github/PULL_REQUEST_TEMPLATE.md Outdated
Show resolved Hide resolved .github/PULL_REQUEST_TEMPLATE.md Outdated
Show resolved Hide resolved .github/PULL_REQUEST_TEMPLATE.md Outdated
Add `docs` section to pull request template
We are able to add further documentation to the release notes by parsing
an additional block at pull requests, for example:

```docs
- Description https://mylink.com
```

This commit adds this block to the pull request template.

Signed-off-by: Sascha Grunert <sgrunert@suse.com>

@saschagrunert saschagrunert force-pushed the openSUSE:docs-pr branch from 6f75b61 to 22bd9c7 Jul 12, 2019

@saschagrunert

This comment has been minimized.

Copy link
Member Author

commented Jul 12, 2019

@saschagrunert -- Coming back to this with a few nits that I've made as code suggestions.

Thanks, I changed it like you suggested. 🙏

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

Thanks!
/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm label Jul 12, 2019

@cblecker

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

/lgtm
/approve
/hold

This looks good.

@justaugustus Would you mind sending a note to k-dev about this? Feel free to pull the hold when you're ready.

@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Jul 12, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cblecker, justaugustus, saschagrunert

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

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 12, 2019

@cblecker -- Will do!

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

/hold cancel
/priority important-soon
/milestone v1.16

@justaugustus

This comment has been minimized.

Copy link
Member

commented Jul 15, 2019

Sent to k-dev, SIG Leads, SIG Release, ContribEx, and Docs: https://groups.google.com/d/topic/kubernetes-dev/91XfKyDyCt4/discussion

@k8s-ci-robot k8s-ci-robot merged commit 391906f into kubernetes:master Jul 15, 2019

23 checks passed

cla/linuxfoundation saschagrunert authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-csi-serial Skipped.
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-e2e-gce-iscsi Skipped.
pull-kubernetes-e2e-gce-iscsi-serial Skipped.
pull-kubernetes-e2e-gce-storage-slow Skipped.
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-node-e2e-containerd Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

@saschagrunert saschagrunert deleted the openSUSE:docs-pr branch Jul 16, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.