-
Notifications
You must be signed in to change notification settings - Fork 194
Allow multiple assignees in NewIssue and EditIssue #336
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
Conversation
src/GitHub/Data/Issues.hs
Outdated
@@ -43,7 +43,7 @@ instance Binary Issue | |||
data NewIssue = NewIssue | |||
{ newIssueTitle :: !Text | |||
, newIssueBody :: !(Maybe Text) | |||
, newIssueAssignee :: !(Maybe Text) | |||
, newIssueAssignees :: !(Maybe (Vector (Name User))) |
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.
is there difference between null
/non-existing field (Nothing) and []
(empty Vector)?
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 believe sending []
causes all assignees to to be removed, and sending null
causes errors from the server. See the notes for assignees
on https://developer.github.com/v3/issues/#edit-an-issue
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 NewIssue
?
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've confirmed that both an empty vector and having the keys missing are accepted by GitHub, so really it's a design question if you would prefer to send "assignees": []
or not.
Oh sorry, answering on my phone and missed the context - it looks as though sending [] would be fine, I'll fix that up tomorrow and test it. |
Ok, I've tested this out and it appears to work. |
Fixes #335
I'm not super familiar with
github
so don't know if there's obvious things I've missed, but these changes appear to work for our usage.