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 Pull request and issue template #1951

Merged
merged 3 commits into from
Nov 2, 2017
Merged

Add Pull request and issue template #1951

merged 3 commits into from
Nov 2, 2017

Conversation

tgraf
Copy link
Member

@tgraf tgraf commented Nov 2, 2017

Fixes: #1792

Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Thomas Graf <thomas@cilium.io>
Signed-off-by: Thomas Graf <thomas@cilium.io>
@tgraf tgraf added the wip label Nov 2, 2017
@tgraf tgraf requested a review from a team as a code owner November 2, 2017 13:07
@tgraf tgraf force-pushed the pull-request-template branch 2 times, most recently from 9faad95 to b7983bd Compare November 2, 2017 13:42
@tgraf tgraf changed the title doc: Pull request template Add Pull request and issue template Nov 2, 2017
@tgraf tgraf added pending review area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. and removed wip labels Nov 2, 2017
@tgraf tgraf requested a review from a team November 2, 2017 13:43
@tgraf tgraf added the release-note/minor This PR changes functionality that users may find relevant to operating Cilium. label Nov 2, 2017
Copy link
Contributor

@aalemayhu aalemayhu left a comment

Choose a reason for hiding this comment

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

This looks good.

https://github.com/cilium/cilium/blob/master/CONTRIBUTING.md for detailed instructions

-->

Copy link
Contributor

@aalemayhu aalemayhu Nov 2, 2017

Choose a reason for hiding this comment

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

will the first lines show up when opening the PR? Looks like a HTML comment.

Copy link
Member Author

Choose a reason for hiding this comment

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

I found this in other repositories that use this. We'll know soon.

Copy link
Contributor

Choose a reason for hiding this comment

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

tried it now. It shows up.


- Cilium version (run `cilium version`)
- Kernel version (run `uname -a`)
- Orchestration system version in use (e.g. Kubernetes 1.7, Mesos, own)
Copy link
Member

Choose a reason for hiding this comment

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

output of kubectl version

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

- Cilium version (run `cilium version`)
- Kernel version (run `uname -a`)
- Orchestration system version in use (e.g. Kubernetes 1.7, Mesos, own)
- Link to relevant artifacts (policies, deployments scripts, ...)
Copy link
Member

Choose a reason for hiding this comment

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

users: "What artifacts?" type an example of a command kubectl get cnp -o json

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe we should defer requiring specific commands to be run until after have the bugtool (#1942) and then include what is left?

Copy link
Member

Choose a reason for hiding this comment

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

That's true... looking through that list, should we also ask the users to paste the ip address show and ip route show?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that's already on the list (ip r and ip a)

Copy link
Member

Choose a reason for hiding this comment

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

I was asking if it should be on the issue template.

Copy link
Member

Choose a reason for hiding this comment

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

I think we can use this for now. As people use this, we can update it to reflect what we think should be added over time.

- Cilium version (run `cilium version`)
- Kernel version (run `uname -a`)
- Orchestration system version in use (e.g. Kubernetes 1.7, Mesos, own)
- Link to relevant artifacts (policies, deployments scripts, ...)
Copy link
Member

Choose a reason for hiding this comment

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

I think we can use this for now. As people use this, we can update it to reflect what we think should be added over time.

@tgraf tgraf merged commit 8d388eb into master Nov 2, 2017
@tgraf tgraf deleted the pull-request-template branch November 2, 2017 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Impacts the documentation, including textual changes, sphinx, or other doc generation code. release-note/minor This PR changes functionality that users may find relevant to operating Cilium.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants