-
Notifications
You must be signed in to change notification settings - Fork 810
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
feat: improve bug template #5086
feat: improve bug template #5086
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5086 +/- ##
==========================================
- Coverage 93.18% 93.17% -0.01%
==========================================
Files 315 314 -1
Lines 8086 8076 -10
Branches 1617 1622 +5
==========================================
- Hits 7535 7525 -10
Misses 551 551 |
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.
Nice improvements! Added a few nits
Co-authored-by: Marylia Gutierrez <[email protected]>
Co-authored-by: Marylia Gutierrez <[email protected]>
@maryliag thanks for the feedback. 🙌 |
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.
Thanks for making the changes!
I just have this nit about the formatting of the steps that you can change or not, otherwise LGTM
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.
Happy to have this merged as is, but I have some nits, suggestions, and Qs about "required: true" usage below.
Co-authored-by: Trent Mick <[email protected]>
Co-authored-by: Trent Mick <[email protected]>
@trentm I addressed some of your comments, two of them are still open - please let me know about your preferred path forward on those. I also updated the screenshot in the title again to reflect the current state 🙂 |
- type: markdown | ||
attributes: | ||
value: | | ||
## Basic Troubleshooting |
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.
As we talked about offline, it might be more effective if we put all of this (useful!) information into a separate dedicated troubleshooting doc, and link to it from the bug report template
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.
Yep - will do, I removed the whole section and will introduce a troubleshooting guide instead. Then I'll introduce a single checkbox along the lines of "I went through the troubleshooting guide but the problem persists" that links to the guide.
I'll also link the guide from README.md
.
- type: textarea | ||
attributes: | ||
label: Runtime and Version | ||
placeholder: Node.js v20.12.1, Node.js v18.18.2, Firefox 130, Chrome 132, ... |
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.
Consider a note in troubleshooting doc for how to get your version (node --version, etc)
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.
I made it optional for now. If someone does not know how to do it they can leave it out. Let's see how well this works when it's optional. If people have problems with finding that info we'll see that bug reports omit the box.
In the follow-up PR to add the troubleshooting doc I'll also update the template here to link to it.
Co-authored-by: Jamie Danielson <[email protected]>
Re-requesting review because the template has changed quite a bit since you all had a look. |
Which problem is this PR solving?
We often get bug reports that don't give full details needed to reproduce.
There's new wording in the placeholder asking for steps to reproduce. There's now more emphasis on asking for reproducer repositories, as well as explaining why this is helpful.
I also added some more emphasis on build instructions and config files for tooling. It's somewhat common that users are very familiar with their stack and assume that we are too. I added a reminder to let them know that JavaScript is a complex ecosystem and that build instructions are immensely helpful in trying to resolve their issue
It's also quite common that users use unsupported runtimes - so I've introduced an optional field where they have to enter their runtime and runtime version so that we can spot it more easily when this is the case.
Finally there's some bugs that only surface on specific Operating Systems - so there's a new optional field for OS and version, which should be relatively easy for the user to provide.
How this looks like
GitHub preview does not fully represent how the issue templates actually looks like, so here's a screenshot of how it will look like (this is on a private repo so asterisks for required fields are missing - they will be there on this repo, since it's public)