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.

Sign up for GitHub

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

Jump to bottom

doc: enhance GitHub template. #1482

Merged
merged 2 commits into from Apr 25, 2017
Merged

doc: enhance GitHub template. #1482

merged 2 commits into from Apr 25, 2017

Conversation

ldez
Copy link
Member

@ldez ldez commented Apr 22, 2017

  • add a pull request template.
  • add quick guides to create issue and PR.
  • enhance existing issue template.

timoreimann
timoreimann suggested changes Apr 22, 2017
View reviewed changes
Copy link
Contributor

@timoreimann timoreimann left a comment •
edited

Choose a reason for hiding this comment

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

Important doc/template enhancement, thanks @ldez. 👍

Left some change requests and a few questions.

.github/ISSUE_TEMPLATE.md Outdated
@@ -0,0 +1,32 @@
How to write a good issue ? Read [this](https://github.com/containous/traefik/blob/master/.github/how-to-write-a-good-issue.md) and the [contributing guide](https://github.com/containous/traefik/blob/master/.github/CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use relative links so that reference don't break if we ever change the repo/hoster.

Copy link
Member Author

Choose a reason for hiding this comment

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

This links are viewed by user who open an issue as text.
If we use relative links, the user can be lost.

Copy link
Contributor

Choose a reason for hiding this comment

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

You're right, I forgot that.

Thanks for pointing out. 👍

.github/PULL_REQUEST_TEMPLATE.md Outdated
@@ -0,0 +1,4 @@
How to write a good pull request ? Read [this](https://github.com/containous/traefik/blob/master/.github/how-to-write-a-good-pull-request.md) and the [contributing guide](https://github.com/containous/traefik/blob/master/.github/CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Relative links here too.

.github/how-to-write-a-good-issue.md
For other type of questions, consider using one of:

- the [Traefik community Slack channel](https://traefik.herokuapp.com)
- [StackOverflow](https://stackoverflow.com/questions/tagged/traefik)
Copy link
Contributor

Choose a reason for hiding this comment

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

I very much like this rule. What I'm wondering though is what kind of consequences are we going to draw from this? I can see two:

  • Close question-style issues right away, pointing to Slack/StackOverflow.
  • Just tolerate them (and maybe leave a "please use Slack/StackOverflow next time" message).

I know the Kubernetes folks do the former. However, they have to be very rigid due to the sheer amount of issues they receive.

Copy link
Member

Choose a reason for hiding this comment

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

I also like it :) I think we can tolerate them, for now. We can change this rule later.

.github/how-to-write-a-good-pull-request.md Outdated
- Make sure all tests pass
- Add tests
- Write useful descriptions and titles
- Keep your branch history clean
Copy link
Contributor

Choose a reason for hiding this comment

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

Not everyone may have the same understand of what this means. Can we be more specific?

.github/how-to-write-a-good-pull-request.md Outdated
- Make sure all tests pass
- Add tests
- Write useful descriptions and titles
- Keep your branch history clean
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we please add a point

  • Address review comments in terms of additional commits; do not amend/squash existing ones unless the PR is trivial.

emilevauge
emilevauge approved these changes Apr 24, 2017
View reviewed changes
Copy link
Member

@emilevauge emilevauge left a comment

Choose a reason for hiding this comment

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

LGTM
Thanks @ldez :)

.github/how-to-write-a-good-issue.md
For other type of questions, consider using one of:

- the [Traefik community Slack channel](https://traefik.herokuapp.com)
- [StackOverflow](https://stackoverflow.com/questions/tagged/traefik)
Copy link
Member

Choose a reason for hiding this comment

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

I also like it :) I think we can tolerate them, for now. We can change this rule later.

vdemeester
vdemeester suggested changes Apr 24, 2017
View reviewed changes
.github/ISSUE_TEMPLATE.md Outdated
@@ -0,0 +1,32 @@
How to write a good issue ? Read [this](https://github.com/containous/traefik/blob/master/.github/how-to-write-a-good-issue.md) and the [contributing guide](https://github.com/containous/traefik/blob/master/.github/CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't those comments be in html comment <!-- … ? Otherwise it's gonna show up in the final issue if ppl don't clean it (and I guarantee that not everybody will clean those). See https://raw.githubusercontent.com/moby/moby/master/.github/ISSUE_TEMPLATE.md 👼

Copy link
Member Author

Choose a reason for hiding this comment

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

It's a gmail filtering technique 😉

.github/PULL_REQUEST_TEMPLATE.md Outdated
@@ -0,0 +1,4 @@
How to write a good pull request ? Read [this](https://github.com/containous/traefik/blob/master/.github/how-to-write-a-good-pull-request.md) and the [contributing guide](https://github.com/containous/traefik/blob/master/.github/CONTRIBUTING.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Same for PRs 👼

vdemeester
vdemeester approved these changes Apr 25, 2017
View reviewed changes
Copy link
Contributor

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

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

LGTM 🐯

@ldez
doc: enhance GitHub template.
ccff8a8 
- add issue and PR guide.
- rewrite templates
@ldez
refactor(bugCmd): update issue template.
972579e 
- fix collides with imported package name.
@ldez ldez merged commit 34b21b9 into traefik:master Apr 25, 2017
@ldez ldez deleted the docs/github-template branch April 25, 2017 10:10
@ldez ldez modified the milestone: 1.3 May 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vdemeester vdemeester vdemeester approved these changes

@timoreimann timoreimann timoreimann approved these changes

@emilevauge emilevauge emilevauge approved these changes

@dtomcej dtomcej Awaiting requested review from dtomcej

Assignees
No one assigned
Labels
area/documentation
Projects
None yet
Milestone
1.3
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@ldez @vdemeester @timoreimann @emilevauge