Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
GitHub infra updates #13542
GitHub infra updates #13542
Changes from 9 commits
4be9ac0
6f41970
61f180d
41eff4f
45cfa26
81d1089
6693155
1bfe844
36f9fcf
2620894
14df3c1
9a408bf
0592074
2c3c29c
1e7d4c5
6474626
7acd2f6
727954a
444222c
ab7cf69
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
This file was deleted.
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.
This is fuzzy to me -- and thus potentially a source of friction for issue filers. If a function is undocumented, that's probably "new." But is an undocumented keyword for an existing function argument supposed to be "new" or "update"?
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.
In the interest of not rocking the boat, we could use the existing template wording of
incorrect
orneeded
if that's preferable.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.
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.
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 think it matters to both parties - it frames the issue with minimal effort (2 mouse clicks). It also lets the user know what we expect of them since the expected responses in the downstream text boxes are conditional (if only the form could be conditional) based on if it's new or an update. (or
incorrect
vsneeded
).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 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 commentThere 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.
This placeholder isn't helpful because you can't copy the link. Can you link this in the label field and remove the placeholder?
This file was deleted.
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.
Label does not match the options (missing "change to existing functionality").
This file was deleted.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Part of a new process to track/triage
External
issues easier. https://github.com/integrations/slack#creating-a-filter when issues withExternal
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.
This file was deleted.