-
Notifications
You must be signed in to change notification settings - Fork 16
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
[ISSUE 31] Create Issue Templates #37
Conversation
Looks like a great start, thanks @daphnegold ! |
Open Q: if we add templates in Github, will they show up in the template dropdown for a new Github issue in Zenhub? I'm not sure, never worked with Zenhub before, I can try to test it out and Google it. :D |
@daphnegold Re: this question:
I haven't used Zenhub directly either, but it looks like there is support for it based on my cursory read of this article One caveat is that the article seems to be referencing the markdown templates not the form-based templates (which is still in beta) I think it would be worth trying and seeing if we can get these to work as is! But maybe fall back on the markdown template if it poses a challenge and folks anticipate using Zenhub to create tickets instead of GitHub? Also sorry for my delinquency on reviewing this PR 🙈 I've been juggling a bunch but promise I will get to it today or tomorrow! |
Looks great. Daphne and I discussed:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @daphnegold Thanks for your patience as I got a chance to review this PR. Overall this is awesome I suggested a few minor changes to the issue templates (mainly to explain the Definition of Done sections) and was thinking that we could also use this PR to introduce a PR template as well.
Highlights
- The forms are really cool!! I need to start adopting them for my personal projects as well.
- Love the addition of the sample ticket as a reference, it would be cool to update the other templates in the future to include a similar reference ticket that folks can model their responses after.
- Yay 🎉 for the proposed testing strategy section, it's great to get folks thinking about how to test their code before they even begin work on the ticket.
Questions/Suggestions
- I added a suggestion to include some help text in the Definition of Done section for ADR and Milestone doc tickets.
- I had some thoughts and a question about default labels (mainly for the internal task template) but was wondering if it's possible to provide a list of suggested labels in the form. It didn't seem like it in the GitHub docs but wasn't sure if you've experimented with this at all.
- Do you think this would be a good opportunity to include a template PR as well? Here's a draft PR template that I personally like to use, but open to other formats as well.
- Can we add a "Summary" section at the top of the internal task ticket just to provide a 1-2 sentence summary of the work required for that ticket?
ISSUE_TEMPLATE/adr.yml
Outdated
- type: markdown | ||
attributes: | ||
value: | | ||
**Example** [Wiki ADR](https://github.com/HHS/grants-api/issues/30) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love the example!!
name: Ticket (Internal Use Only) | ||
description: Tracking our work through tickets | ||
title: "[Ticket]: " | ||
labels: ["backlog"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not opposed to having us default to this label, but curious if we might actually wind up removing it most of the time depending on when tickets are created -- especially if we're usually creating task tickets for work that is being planned in the current/next sprint.
Is it possible to provide the person creating the task with a list of suggested tags that they can choose from? If so, I might recommend suggesting the topic:
labels:
topic: backend
topic: frontend
topic: data
topic: research
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this label is a remnant from when I set up an automation in Github Projects to put anything labeled backlog
into our project in the backlog column. I'm happy for it to be whatever!
I don't believe it's possible to add non-static labels via form input at this time, but I can check further. The output is markdown and you can add whatever labels you'd like in the same ways you would on any issue via the side-panel wizard. Is there a static label you'd like added by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I figured I might be asking too much with label suggestions. Since the internal team will have the ability to select labels once it's been created, I'm not sure we need a default label unless we wanted to have a task
label.
But I'd be fine with leaving it blank!
id: testing | ||
attributes: | ||
label: Describe the proposed testing strategy | ||
description: A clear and concise description of what testing is required. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oooh love this!! ❤️
Co-authored-by: Billy Daly <[email protected]>
Co-authored-by: Billy Daly <[email protected]>
@widal001 thank you so much for the thoughtful comments! I appreciate it lots! We have a simple PR template that is part of the Flask repo, but let's add one here that meets our desires! This one looks fantastic. The one thing I'll say is that because the issue + PR are so intricately tied in Github/Zenhub systems, I think having the individual have to cut and paste as little as possible into the PR description and minimizing repeating themselves would be helpful. So I like to be thoughtful about only including exactly what is necessary. 99% of the time I've noticed people just delete almost the entire PR template. On another note, I am a little hyped to get some info auto populated into PRs, like issue # and title and stuff. Someday. :D |
Yes! Love this instinct. The sections that I find most helpful as a reviewer are and think would be good to include at a minimum are:
But feel free to propose another PR template that you all find helpful. Linking the associated issue is also helpful, but it does feel like something we could maybe set up a GH action for if folks are following the correct naming convention with branches. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@daphnegold thanks for making these changes! Unless other folks wanted to review I say let's get this merged in and start reaping the benefits of beautifully templated issues and PRs 😄
## Summary | ||
Fixes #{ISSUE} | ||
|
||
### Time to review: __x mins__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh love this!!
> What was added, updated, or removed in this PR. | ||
|
||
## Context for reviewers | ||
> Testing instructions, background context, more in-depth details of the implementation, and anything else you'd like to call out or ask reviewers. Explain how the changes were verified. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool this feels much more flexible than "Instructions to review", great addition
Fixes #31
Adding some baseline templates to a draft PR as a starting point.
Example of ADR form template
Example of Milestone form template