Skip to content

Add more specific GitHub issue templates #18161

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

Merged
merged 8 commits into from
Aug 13, 2020

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Aug 3, 2020

PR Summary

Closes: #18160

  • Added a feature request template that will automatically be tagged with the New Feature label.
  • Moved ISSUE_TEMPLATE.MD to ISSUE_TEMPLATE/bug_report.md so that both issue templates will show up as options for opening a new issue.

The feature request template is an almost exact copy of the jupyterlab template but I wrote the jupyterlab version so I think it's fine to just copy it w/o attribution.

ping @story645

PR Checklist

  • na Has Pytest style unit tests
  • na Code is Flake 8 compliant
  • na New features are documented, with examples if plot related
  • na Documentation is sphinx and numpydoc compliant
  • na Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • na Documented in doc/api/next_api_changes/* if API changed in a backward-incompatible way

I also moved the ISSUE_TEMPLATE.md to ISSUE_TEMPLATE/bug_report.md so that both feature requests and bug report templates will show up after clicking the new issue button
@story645
Copy link
Member

story645 commented Aug 3, 2020

Thanks for opening this!
For reference for other folk looking at this PR, went back into the MPL meeting notes and the discussion around this was to adopt the sklearn model (pandas does something similar/ it seems to be kinda standard). These are what was agreed upon:

  • Bug / Crash: Something is happening that clearly shouldn’t happen. Wrong results as well as unexpected errors from estimators go here.
  • Cleanup / Enhancement: Improving performance, usability, consistency.
  • Documentation: Missing, incorrect or sub-standard documentations and examples.
  • New Feature: Feature requests and pull requests implementing a new feature.

@story645 story645 self-assigned this Aug 3, 2020
@jklymak
Copy link
Member

jklymak commented Aug 3, 2020

@story645 are you asking for all these categories to be implimented? Whats the next step here?

@story645
Copy link
Member

story645 commented Aug 3, 2020

No, just the Bug and New Feature is great! I'm just putting this here cause we already agreed on it on a call and it's kind of a pain to pull up.

* Add an option so that when [...] [...] will happen
-->

### Additional context
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Additional context
### Additional context and prior art

---

<!--
Welcome! Thanks for thinking of a way to improve Matplotlib. If this solves a problem for you, then it probably solves that problem for lots of people! So the whole community will benefit from this request.
Copy link
Member

Choose a reason for hiding this comment

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

please fix the line breaks....

I'd just shorten to Thanks for thinking of a way to improve Matplotlib. and leave off the rest.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please fix the line breaks....

Do you mean inside the paragraph or do you mean how the text is positioned relative to to the comment lines?

Copy link
Member

Choose a reason for hiding this comment

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

Just we wrap most lines at 79 characters. Of course not as big a deal with a markdown file, but...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

got it. Would you be open to avoiding that rule in the issue templates? As they are prose rather than code I think it makes more sense for the github issue editor/browser to handle the reflow/wrapping. For example the line wrapping at 79 characters in the current issue template feels unnatural to me:
image

There are a few other places in the feature request template where I went over 79 lines, and the existing bug_report template seems to have inconsistent enforcement of this.

Copy link
Member

Choose a reason for hiding this comment

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

I don't feel strongly about this, but others do. I think unless there is a good reason (i.e. it'll make horrible line continuations) we stick to 79 characters.

@ianhi
Copy link
Contributor Author

ianhi commented Aug 4, 2020

I added a template for each of @story645's suggestions. I used the Enhancement label for the enhancement as that felt most natural but it does not seem to exist as a label currently. If there is a better label to use please let me know and I will change it.

@story645
Copy link
Member

story645 commented Aug 4, 2020

Thanks! Maintenance is probably the best fit for those sorts of things.
(Sorry didn't read the first time if you're getting some conflicting emails)

@dopplershift
Copy link
Contributor

I don't want to bikeshed this PR to death, but to me "Enhancement" != "maintenance". You don't enhance your car by changing its oil, but it is maintenance. I'd say add the new label, but I'm content to be told how wrong I am. 😉

@story645
Copy link
Member

story645 commented Aug 4, 2020

Ryan these are for clean ups & enhancements, which I think folks have been tagging maintenance? There's a seperate issue template for new features and we just discussed on the call how we don't want a standalone enhancement tag cause nobody knew what it was for.

@jklymak
Copy link
Member

jklymak commented Aug 4, 2020

I would not include the "Enhancement.md" at all. We already have "Bug or issue" and "New Feature", and its hard to think of an "enhancement" request not covered under one of those two templates.

@story645
Copy link
Member

story645 commented Aug 4, 2020

From the July 20th meeting notes

replace "enhancement" with "new feature"

  • new feature makes new API
  • performance enhancement & maintenance clean up code

& this would be for stuff tagged https://github.com/matplotlib/matplotlib/issues?q=is%3Aissue+is%3Aopen+label%3Amaintenance

@jklymak
Copy link
Member

jklymak commented Aug 4, 2020

I'm confused about what you are trying to say @story645. Are you arguing for keeping enhancement.md in this PR or removing it?

@story645
Copy link
Member

story645 commented Aug 4, 2020

I'm confused about what you are trying to say @story645. Are you arguing for keeping enhancement.md in this PR or removing it?

keeping it as a template for maintenance type issues

@jklymak
Copy link
Member

jklymak commented Aug 4, 2020

Which recent issues do you wish the user had used the proposed "Maintenance" template for? Because I don't see the benefit from the user's point of view.

Copy link
Member

@story645 story645 left a comment

Choose a reason for hiding this comment

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

minor nitpicks but can also be handled by anybody else when they tweak the templates.

@ianhi
Copy link
Contributor Author

ianhi commented Aug 4, 2020

Is there any harm in having 1~2 more issue templates? The difficulty here seems to be derived from the lumping of enhancement and cleanup together. How about splitting them up, giving a breakdown like this:

  • feature request new api
  • enhancement performance improvement or other small improvment
    • I often feel weird calling a request for small improvements a "feature request" so having a dedicated enhancement makes sense to me.
  • clean-up correct inconsistencies

I think the initial issue I raised for this is a good example of enhancement while the following would be a good example of cleanup:

The acceptable usage of gitter is different on the matplotlib website:
image

vs on the readme:
image

Co-Authored-By: hannah <story645@users.noreply.github.com>
@QuLogic QuLogic changed the title add feature request template Add more specific GitHub issue templates Aug 5, 2020
@QuLogic
Copy link
Member

QuLogic commented Aug 5, 2020

Can we also add something like "I don't know what I want" that redirects to Discourse?

@story645
Copy link
Member

story645 commented Aug 5, 2020

@QuLogic some other projects make a "question/support/other" type template that explicitly says something like the issue tracker isn't for this sort of thing so we're most likely going to close this issue and you should use one of our other channels instead.

Here's the scikit-learn version:
https://www.github.com/scikit-learn/scikit-learn/tree/master/.github%2FISSUE_TEMPLATE%2Fusage_question.md

@ianhi
Copy link
Contributor Author

ianhi commented Aug 9, 2020

So in summary the conclusion is to have the following:

  • bug report
  • feature request new api
  • documentation Missing, incorrect or sub-standard documentations and examples.
  • enhancement performance improvement or other small improvment
  • clean-up correct inconsistencies
  • "question/support/other redirect to Discourse

Is that a fair assement? If yes, shall I set the label for enhancement to enhancement and use the maintenance label for cleanup? I think this may require action by someone to actually create the label.

@QuLogic
Copy link
Member

QuLogic commented Aug 10, 2020

I think we might have crossed wires a bit. We don't have an enhancement label and in fact, we deleted it recently as it seem New feature was more indicative of what it meant.

For performance and small improvements, we can lump that together with cleanup and label it maintenance. While we do have a separate performance label, the number of such PRs is much smaller, so I think it's not a great burden to add that label after the PR is opened. If it does turn out to be a burden, we can always add another template type later.

And to answer another question, Enhancement != Maintenance, but the difference here is a matter of scope. If it's an Enhancement that requires a What's new entry (maybe also for API notes too), then the Enhancement is a New feature. If it's an Enhancement that makes the library 'better' (for some definition of 'better'), but isn't user-visible, we'll call it Maintenance.

@QuLogic
Copy link
Member

QuLogic commented Aug 10, 2020

In terms of concrete to-do:

  • merge enhancement and cleanup and call it maintenance (with that label), but try to avoid calling it an Enhancement
  • add question/support/other redirect to Discourse

Copy link
Member

@QuLogic QuLogic left a comment

Choose a reason for hiding this comment

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

Not sure if labels are case-sensitive.

case sensitivity. Also put discourse link before SO to prioritize it.
I belive that the line at the top would have stopped github from picking this up as a valid template.
@ianhi
Copy link
Contributor Author

ianhi commented Aug 12, 2020

This is the only one with a period.

not only that but there was an extra line at the top of the file that I believe would have caused github to not pick up the template. I think they all should be good now

Comment on lines +5 to +6
# This template is based on the scikit-learn template
# https://github.com/scikit-learn/scikit-learn/blob/8e61534f1087703f476414d8dbd3688282f8eebf/.github/ISSUE_TEMPLATE/usage_question.md
Copy link
Member

Choose a reason for hiding this comment

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

Going back and forth on whether this needs to be in here...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It won't show up when someone makes a new issue as it is a comment in the yaml frontmatter rather than in the markdown body. See example here: https://github.com/ianhi/git-expt/issues/new?labels=Question&template=test.md and the source file https://raw.githubusercontent.com/ianhi/git-expt/master/.github/ISSUE_TEMPLATE/test.md

Copy link
Member

Choose a reason for hiding this comment

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

thanks for the clarification.

@story645 story645 merged commit 1d99132 into matplotlib:master Aug 13, 2020
@story645
Copy link
Member

Thank you @ianhi for knocking something off my to do list 😉!

@ianhi
Copy link
Contributor Author

ianhi commented Aug 13, 2020

Thanks @story645 for helping get it through! I really appreciate having nice templates when I open an issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add feature request template
5 participants