Allow RemoveReviewers to remove only teams

[nagl-resourcely]

What?

Fixes a JSON marshaling problem that causes spurious 422s when calling RemoveReviewrs with TeamReviewers but no Reviewers.

Background

The github API requires "reviewers". It permits "reviewers": [] but not "reviewers": null. Try it:

echo "Show the problem"
curl -L \                                                            
  -X DELETE \   
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer ${PAT}" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/${OWNER}/${REPO}/pulls/${NUMBER}/requested_reviewers \
  -d '{"team_reviewers":["the_senate"]}' 
{
  "message": "Invalid request.\n\n\"reviewers\" wasn't supplied.",
  "documentation_url": "https://docs.github.com/rest/pulls/review-requests#remove-requested-reviewers-from-a-pull-request",
  "status": "422"
}

echo "What if we just removed omitempty?"
curl -L \
  -X DELETE \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer ${PAT}" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/${OWNER}/${REPO}/pulls/${NUMBER}/requested_reviewers \
  -d '{"reviewers": null, "team_reviewers":["the_senate"]}'
{
  "message": "Invalid request.\n\nFor 'properties/reviewers', nil is not an array.",
  "documentation_url": "https://docs.github.com/rest/pulls/review-requests#remove-requested-reviewers-from-a-pull-request",
  "status": "422"
}

echo "Does the empty list work?"
curl -L \
  -X DELETE \
  -H "Accept: application/vnd.github+json" \
  -H "Authorization: Bearer ${PAT}" \
  -H "X-GitHub-Api-Version: 2022-11-28" \
  https://api.github.com/repos/${OWNER}/${REPO}/pulls/${NUMBER}/requested_reviewers \
  -d '{"reviewers": [], "team_reviewers":["the_senate"]}'
{
  "url": ...<spam>,

Testing?

There's a new unit test. Without the 2nd commit, it fails with:

    pulls_reviewers_test.go:181: request Body is {"team_reviewers":["justice-league"]}
        , want {"reviewers":[],"team_reviewers":["justice-league"]}

[gmlewis]

Thank you, @nagl-resourcely!
One nit, otherwise LGTM.

Then we will be ready for a second LGTM+Approval from any other contributor to this repo before merging.

[nagl-resourcely]

Ty for expressing the nit as a diff =)

[gmlewis]

Thank you, @nagl-resourcely !
LGTM.

Awaiting second LGTM+Approval from any other contributor to this repo before merging.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.02%. Comparing base (2b8c7fa) to head (651c746).
Report is 163 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3337      +/-   ##
==========================================
- Coverage   97.72%   93.02%   -4.71%     
==========================================
  Files         153      172      +19     
  Lines       13390    14860    +1470     
==========================================
+ Hits        13085    13823     +738     
- Misses        215      944     +729     
- Partials       90       93       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tomfeigin]

LGTM

[gmlewis]

Thank you, @tomfeigin !
Merging.