Add Issue and PR templates to the project #591

lilic · 2018-10-10T12:58:56Z

As per the suggestion in the issue #585 I opened, we went with using the .github directory to store all the issue and PR templates. Inspired by https://github.com/babel/babel.

Suggestions are very welcome, especially for the PR template.

Closes #585

shawn-hurley

LGTM just a couple of ideas.

shawn-hurley · 2018-10-10T13:24:48Z

.github/ISSUE_TEMPLATE/Bug_report.md

+
+
+**Environment**
+* operator-sdk version:


We may need to have two version here (maybe?) 1. is the binary's version and 2. the version from Gopkg.Toml.

Gopkg.toml exists only for go type operators, right? Might need to clarify that that version only applies to go operators.

There could also be a potential mismatch of the version from the operator-sdk binary and the base image for the ansible operator that a user is using? Don't know how to capture that though.

@shawn-hurley The ansible-operator logs should show the version of the vendored sdk.

INFO[0000] Go Version: go1.10.3 INFO[0000] Go OS/Arch: linux/amd64 INFO[0000] operator-sdk Version: 0.0.6+git INFO[0000] Starting to serve on 127.0.0.1:8888

We could ask them to check the Gopkg.toml for a Go type operator. And the logs of their operator for an ansible type operator to check the version of the sdk. Assuming they can run the operator.

shawn-hurley · 2018-10-10T13:26:48Z

.github/ISSUE_TEMPLATE/Bug_report.md

+  insert output of `kubectl version` here
+
+* Kubernetes cluster kind: 
+


I think we should add a field about using ansible/go/helm. Don't have a good idea on wording though. Maybe Are you writing your operator in ansible, go, or helm?.

shawn-hurley · 2018-10-10T13:33:47Z

.github/ISSUE_TEMPLATE/Question.md

+
+---
+
+## Question


It might be nice to understand the context of the question.

something like Are you asking about community best practices, how to implement a specific feature, or about general context and help around the operator-sdk?

Thoughts?

Yeah, that could make it easier for us when we're quickly looking through recent issues.

@shawn-hurley where were you thinking of putting the above text? As a comment or as a section?

I was thinking it would be a section similar to the Operator-SDK Version: above. I think we may want to put this before the Question header?

@shawn-hurley Done, let me know if this is what you had in mind?

joelanford · 2018-10-10T15:35:14Z

.github/PULL_REQUEST_TEMPLATE.md

@@ -0,0 +1,5 @@
+<!--
+Before making a PR, please read our contributing guidelines https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD


Should we ask submitters to make sure their branches are rebased to the latest revision of the destination branch before submitting?

I would say yes. Especially given that we vendor the contributor's branch as the source in our e2e test.

If their branch is significantly behind the master there can be conflicts.

Yeah, that would be useful considering that the tests tend to fail a lot if they are not recently rebased simply due to the way we have to handle some of our tests due to dep.

hasbro17 · 2018-10-10T23:21:00Z

.github/PULL_REQUEST_TEMPLATE.md

+Before making a PR, please read our contributing guidelines https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD
+--!>
+
+Patch: Bug Fix? 


Would this show up as is, in the PR template?

This was more like a suggestion, no clue what to add here TBH. Any ideas? If none will just leave the commented out section.

What we put in here probably depends on whether we require an open issue for a PR. My reading of the CONTRIBUTING.MD file leads me to believe we don't.

If that's true, we'll probably want to borrow some from the bug fix and feature request templates to ask the submitter to provide details about what the PR changes and the motivation for those changes.

@joelanford good idea, added some more information. Feel free to suggest some more things, no clue if this makes sense currently or how it will look. :)

lilic · 2018-10-15T09:26:36Z

Any other suggestions @shawn-hurley @hasbro17 @joelanford @AlexNPavel. We can also adjust when this is merged and when we see how it looks like.

shawn-hurley · 2018-10-15T11:59:27Z

LGTM

hasbro17 · 2018-10-15T20:51:12Z

One thing we could add to the PR template is the reminder to update the CHANGELOG accordingly.
https://github.com/operator-framework/operator-sdk/blob/master/CHANGELOG.md#unreleased

We haven't done a good job of keeping that up to date ourselves so this will be a good reminder.

joelanford · 2018-10-15T20:56:15Z

LGTM

hasbro17

Hold off on merging until the refactor branch is merged into the master.
https://github.com/operator-framework/operator-sdk/tree/refactor/rebase-master-add-AO-commands

lilic · 2018-10-16T09:18:25Z

One thing we could add to the PR template is the reminder to update the CHANGELOG accordingly.

I personally think that should be done on release as a final PR before the release is cut. Not sure our external contributors would want to keep that updated.

hasbro17 · 2018-10-16T22:08:12Z

.github/ISSUE_TEMPLATE/Bug_report.md

+
+* Kubernetes cluster kind: 
+
+* Are you writing your operator in ansible, go, or helm?


Nit: we don't have helm yet. So probably remove helm.

hasbro17 · 2018-10-16T22:15:03Z

I personally think that should be done on release as a final PR before the release is cut

@lilic That's kind of what I wanted to remedy. It's easy to forget changes made at the time of cutting a release when you have to go through the commit/PR history to decide what changes (added/updated/removed) are of note.
Updating the CHANGELOG along with the PR helps us keep it up to date.

Although I agree that external contributors might not always know what should go in the CHANGELOG. I wanted it more of a reminder for us. But it's not a big issue, we can leave it out.

hasbro17

LGTM after nit

lilic · 2018-10-17T06:40:11Z

Updating the CHANGELOG along with the PR helps us keep it up to date.

Agreed!

I noticed we already have that in the CONTRIBUTING doc https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD#documentation, but lets add it to the PR template in the future if we find we forgot to do it often! :)

These files are used as issue templates.

This file will be used when creating default issue.

lilic requested review from spahl, joelanford, shawn-hurley, AlexNPavel, hasbro17 and estroz October 10, 2018 12:58

openshift-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 10, 2018

shawn-hurley reviewed Oct 10, 2018

View reviewed changes

joelanford reviewed Oct 10, 2018

View reviewed changes

hasbro17 reviewed Oct 10, 2018

View reviewed changes

hasbro17 suggested changes Oct 15, 2018

View reviewed changes

hasbro17 reviewed Oct 16, 2018

View reviewed changes

hasbro17 approved these changes Oct 16, 2018

View reviewed changes

lilic force-pushed the lili/add-issue-pr-template branch from b3065ec to 248c61e Compare October 17, 2018 06:42

lilic added 3 commits October 17, 2018 08:50

.github: Add ISSUE_TEMPLATE directory

3fe9955

These files are used as issue templates.

.github: Add default issue template file

27f06dd

This file will be used when creating default issue.

.github: Add PR template

f225b9b

lilic force-pushed the lili/add-issue-pr-template branch from 248c61e to f225b9b Compare October 17, 2018 06:51

lilic merged commit dcd51eb into operator-framework:master Oct 17, 2018

lilic deleted the lili/add-issue-pr-template branch October 17, 2018 07:06

		insert output of `kubectl version` here

		* Kubernetes cluster kind:

		@@ -0,0 +1,5 @@
		<!--
		Before making a PR, please read our contributing guidelines https://github.com/operator-framework/operator-sdk/blob/master/CONTRIBUTING.MD


		* Kubernetes cluster kind:

		* Are you writing your operator in ansible, go, or helm?


		---

		## Question

Add Issue and PR templates to the project #591

Add Issue and PR templates to the project #591

Uh oh!

Conversation

lilic commented Oct 10, 2018

Uh oh!

shawn-hurley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexNPavel Oct 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lilic commented Oct 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shawn-hurley commented Oct 15, 2018

Uh oh!

hasbro17 commented Oct 15, 2018

Uh oh!

joelanford commented Oct 15, 2018

Uh oh!

hasbro17 left a comment

Choose a reason for hiding this comment

Uh oh!

lilic commented Oct 16, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hasbro17 commented Oct 16, 2018

Uh oh!

hasbro17 left a comment

Choose a reason for hiding this comment

Uh oh!

lilic commented Oct 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

AlexNPavel Oct 10, 2018 •

edited

Loading

lilic commented Oct 15, 2018 •

edited

Loading

lilic commented Oct 17, 2018 •

edited

Loading