GitHub infra updates #13542
GitHub infra updates #13542
Conversation
|
Would it be ok if we lose the exclamation points? |
| issue_url=${{ github.event.issue.html_url }} | ||
| gh issue edit ${issue_url} --add-label "Needs Triage,External" | ||
|
|
||
| - name: add-comment-to-issue |
There was a problem hiding this comment.
Personally I am not in favor of autoresponders if their text is so simple. It adds noise to the conversation that I don't think is helpful. If it were "smart" and pointed to a contributing guide for the relevant component (libcudf/cudf Python) or linked to potentially related issues, then perhaps.
There was a problem hiding this comment.
Agree in that auto-responders are not my favorite. Unfortunately for the Slack integrations to notify us it requires a triggering event, and that's only either Issue creation, or comment. Since the GHA to label happens after issue creation, we need to make the comment to have it fire.
I think we could have the GHA delete the comment immediately after posting, but that might fire some confusing emails to filers.
There was a problem hiding this comment.
What Slack integration are you talking about? I already get GitHub notifications via Slack for every new issue that is filed.
There was a problem hiding this comment.
Part of a new process to track/triage External issues easier. https://github.com/integrations/slack#creating-a-filter when issues with External exist they can go into specific channels to ensure timely response/handling.
The goal is to reduce noise of all issues/comments and only focus on a small subset.
|
Re: |
Co-authored-by: Bradley Dice <bdice@bradleydice.com>
|
I batch merged all of your changes @bdice, thank you! I'll go through the content of the files again either tonight or tomorrow morning and ensure alignment with the suggestions we merged. |
davidwendt
added
3 - Ready for Review
There was a problem hiding this comment.
Thanks! I have some comments that are similar to @bdice's around how much of this boilerplate is for our benefit, and how much is really for the reporter. I would like it to be low-friction for people to report bugs and (at least from my point of view), when I try and report a bug to a project with a complicated form I often just give up.
| - type: dropdown | ||
| id: new_or_correction | ||
| attributes: | ||
| label: Is this for new documentation, or an update to existing docs? |
There was a problem hiding this comment.
Does it matter whether the request is for "new" or "update" from the point of view of the reporter? It might matter for us, but that is an internal "implementation" detail.
| options: | ||
| - Critical (currently preventing usage) | ||
| - Medium | ||
| - Low (would be nice) |
There was a problem hiding this comment.
My feeling is that this more likely than not to just cause annoyance. We are going to triage importance anyway and if we decide that, in our estimation, a user's "critical" issue is in fact low priority what then?
|
I’ve only dealt with form issues on a couple of projects but I prefer to skip them and just make a blank issue where I can write freely. I prefer to frame a problem/proposal in my own terms. As an internal dev, I’m not expecting to use these forms very often. I think the external triage is the best part of this PR. Can we achieve that benefit without the friction of forms? |
|
The GHA for labeling is entirely independent and could be spun into a separate PR. If you'd prefer that to expedite it, we could do that. I do think the friction here is worth it though because consistent, easy to fill out templates make things better for both developers who choose to use them, and users in the long run. I don't propose we disable blank issues or anything like that! The less back-and-forth we need to have to get to the root of an issue the faster we can triage and resolve. Clear templates with required fields and easy to follow steps help get us there. As an aside, for proposals and such, I'm a big proponent of GitHub Discussions to be honest. I didn't submit that change here since I thought it was too big do to without...discussion. IMO issues should be actionable tasks or trackers that support the library and proposals/questions don't fit in perfectly with the newer project board management (when is a proposal or question |
We have been discussing this as an option as part of broader thinking on where we should be asking people to ask usage questions (currently in too many places is the general feeling). |
There was a problem hiding this comment.
Thanks @jarmak-nv! I am happy here with all the wording now. I think @bdice's change requests have all been dealt with but will let him confirm.
|
@bdice do you still have concerns here? |
|
Was hoping to get at least one more perspective here, but also we'll get plenty of perspectives if it goes live and someone has concerns 😅 |
|
@wence- @jarmak-nv I dismissed my request for changes to unblock this. I would like feedback from PICs, @GregoryKimball spends a lot of time triaging and would be good to consult here. |
|
To any reviewers, sometimes with these it's easiest to just try the form - you can do that here: https://github.com/jarmak-nv/cudf/issues/new/choose |
| - type: dropdown | ||
| id: new_or_improvement | ||
| attributes: | ||
| label: Is this a new feature, an improvement, or a change to existing functionality? |
There was a problem hiding this comment.
Label does not match the options (missing "change to existing functionality").
There was a problem hiding this comment.
I'm interested in automatically labeling "External" issues.
As far as the templates, I don't like that I can't erase all the template to start with a blank issue - is there a way to do that? I write a lot of story issues and it seems like this change will prevent me from doing that (!!) It's pretty hard to find the "blank issue" option. This option is critical for complex and important issues. We should not be hiding it behind more than a "select-all erase"
I'd like to consider excluding the "Feature Request" form from this change. The "Bug report" template has some nice properties.
For "Documentation request" could we have a radio select between incorrect and needed that populates a text box with the correct half of the existing form content?
## Report incorrect documentation
**Location of incorrect documentation**
Provide links and line numbers if applicable.
**Describe the problems or issues found in the documentation**
A clear and concise description of what you found to be incorrect.
**Steps taken to verify documentation is incorrect**
List any steps you have taken:
**Suggested fix for documentation**
Detail proposed changes to fix the documentation if you have any.
---
## Report needed documentation
**Report needed documentation**
A clear and concise description of what documentation you believe it is needed and why.
**Describe the documentation you'd like**
A clear and concise description of what you want to happen.
**Steps taken to search for needed documentation**
List any steps you have taken:
| - type: dropdown | ||
| id: new_or_correction | ||
| attributes: | ||
| label: Is this for new documentation, or an update to existing docs? |
There was a problem hiding this comment.
I would like to remove this dropdown. As a filer, I don't think it adds enough value for the 2 click friction. I updated my feedback in my main review comment
|
@GregoryKimball The blank issue is hidden. If you look at https://github.com/jarmak-nv/cudf/issues/new/choose (@jarmak-nv merged this PR to his fork so you can see what it does), there is a link at the bottom. See the red highlight in the screenshot below. GitHub always allows users to choose this, regardless of what forms the repo has.
|
Co-authored-by: Bradley Dice <bdice@bradleydice.com> Co-authored-by: Gregory Kimball <gkimball@nvidia.com>
Definitely not looking to force a change that makes the teams' life harder! Just curious - in the feature/stories you submit, is there a structure you follow? The goal here is the standardize issues so they always include a minimum amount of necessary information to make it something any team member could act upon. I'm not entirely against not using the feature req form, or maintaining both an internal feature req .md template and an external facing form, I just want to see if there's a way to standardize with minimum friction. If not, totally accept that.
I wish! Contextual awareness isn't a thing in form templates. It would be awesome, but yeah 😞 Syntax for ref. |
|
Thank you @jarmak-nv for your feedback. Feature request form: let's please not change this for now. Features are hard to standardize and I like the current template. To answer your question, here are some example story issues I've written: #11844 #13159 #13048 Bug report form: This seems like a good form, except we MUST automatically create a hidden block for the log output and environment details. Please see this example: jarmak-nv#1. Also let's please make "version" an optional field. Documentation request form: I'm inclined to keep the existing template for this one as well, unless we can find a way to make the difference between |
|
^ Yes please. I’m in favor of the form structure for bugs but not features/doc issues. Plain text helps with expression for features/docs. Structuring a bug report ensures that we get enough information AND helps with triaging the root cause. |
|
I disagree that forms restrict doing any of the stories you've posted. It provides full markdown editors for each markdown field that you can expand to be the size of the browser windows. The layout we currently have might, but that is easy enough to change. That being said - I'm not going to push it. I'll revert the docs and feature request. Happy to revisit this any time if things change on your end! |
|
OK great, @jarmak-nv let's please do a deeper dive on the "Bug report" form. The log output and environment details can be very large. Good bug reports put this information into a markdown detail block such as: Is it possible to add this pattern to the form template? |
|
Took way too much time for me to get back to this, apologies.
I tested a bunch of ways to make it entirely seamless to no avail, but it does pre-fill the response now so they just need to paste it within the html tags and it works. Would be nice if one of the shell renderers was default collapsed but they are not. |
|
Thank you @jarmak-nv for continuing to experiment with this.
Click here to see environment details
|
I believe this is a restriction on the github commenting system (which stores individual comments in a database with up to 64K 4-byte characters). Is your print_env output particularly long? What does That said, for large outputs I suspect the recommended thing is to upload things (rather than copy-paste). So we could just adapt the text to say attach rather than paste. |
Not hard to do so, but won't we always ask that if someone files without the info? If there's a question we will ask every time, and it's easy for the user to answer, IMO we should require. We could include in the description that I suppose since we're asking for print_env details it will be there, but I'm imagining not all users will go to that length.
Agree the conditional should be in the description above the box, changed. I left in the environment location because I think it's relevant to the user and makes it faster to enter. I did change the line to
Yeah will echo what @wence- said, I don't have any control over the limit unfortunately. I can, and will, bring up the concern he raises in their feedback Discussions though. In a typical debugging session, do you think it's more relevant to have the conda packages section, or everything above it? I think we could ask for a copy/paste of either/or of those, or have them attach as suggested to make sure they don't hit the limit. |
|
As noted in #13504 since there's no ETA on resolving the length issue present in form templates, I've reverted the bug report template. The |
|
Closing, will reopen new PRs:
|
Description
closes #13504 (ended up doing just 1 PR to minimize CI/CD time and review time.)
This PR does a few things supporting updated GH infrastructure:
RAPIDSAIorg withNeeds TriageandExternalTODO BEFORE MERGE:
ExternallabelChecklist