Gracefully handle r? of invalid user
#300
Conversation
Don't crash; instead post a comment saying that the user couldn't be assigned. Also handle `r? @ghost` properly: treat it as clearing all assignees but still do the other things that highfive normally does (e.g. setting `S-waiting-on-review`).
|
Hmm, I'm not sure how to update the tests. There are some other spots that reference the ❯ rg "'assignee'"
highfive/newpr.py
426: self.payload['issue', 'assignee'] \
427: and commenter == self.payload['issue', 'assignee', 'login']
highfive/tests/test_newpr.py
911: 'assignee': None,
933: payload._payload['issue']['assignee'] = {'login': assignee} |
|
Before this PR if an unassignable person was mentioned in a The easiest fix for this is to continue raising the exception if the person is unassignable, but show the error message before if the user is |
|
Skipping the
But won't that also skip showing the submodule warning? I feel like we should treat |
|
r? @ghost PRs should be treated as "null don't look" IMO, not assigned, not pinged, etc. -- minimal footprint. e.g. if I'm just opening for a perf run, I don't want to ping if I've changed submodules etc. If you do want to ping or get wg-triage to ping, do r? @Mark-Simulacrum (for example for me), i.e., assign yourself. |
Apparently people prefer the old `r? @ghost` behavior. Now the only difference is that `r? @ghost` will unassign anyone already assigned (e.g. if you use `r? @ghost` after highfive has already assigned someone).
| self.api_req( | ||
| "PATCH", issue_url % (owner, repo, issue), | ||
| {"assignee": assignee} | ||
| {"assignees": assignees} |
There was a problem hiding this comment.
This PR also updates highfive to no longer use assignee: it's deprecated, and I don't we would be able to clear assignees easily otherwise. I'm not sure what the old behavior was when there were multiple assignees though. When looking at the paylod, I just look at the first assignee.
There was a problem hiding this comment.
Actually now I'm just checking if the assignee is a member of assignees.
|
Ugh, I miss you |
3 participants
Don't crash; instead post a comment saying that the user couldn't be
assigned.
Also handle
r? @ghostproperly: treat it as clearing all assignees butstill do the other things that highfive normally does (e.g. setting
S-waiting-on-review).r? @pietroalbini