We recently received a bug report that implied that for certain
payloads, the `requested_reviewers` key was empty whereas a
singular `requested_reviewer` key containing one reviewer's
information was present in its stead. Naturally, this raised
some not so pretty IndexError exceptions.
After some investigation and generating a few similar payloads,
I discovered that in every case both the `requested_reviewers`
and the `requested_reviewer` keys were correctly populated, so I
had to manually edit the payload to reproduce the error on my end.
My guess is that this anomaly goes back to when GitHub's reviewer
request feature was new and didn't support requesting multiple
reviewers, and that the singular `requested_reviewer` key could
possibly just be there for backwards compatibility or might just
be mere oversight. Either way, the solution here is to look for the
plural `requested_reviewers` key, and if that is empty, fall back
to the singular `requested_reviewer` key.
Some of the page build message code had insufficient test coverage.
I looked at generating the payloads that would allow me to test
the lines of code in question, but it was too much work to
generate the payloads and this seemed like a vague event anyway.
So I just rewrote the logic so that the lines missing
coverage are implicitly covered.
This is a part of our efforts to get this webhook's coverage
up to 100%.
Note that apart from just testing an uncovered line of code, this
commit also fixes a minor bug in the code for messages about issue
comment deletion and editing.
This is a follow-up in response to Tim's comments on #9951.
In instances where all messages from a GitHub integration are
grouped under one user specified topic (specified in the URL), we
should include the title of the issue/PR in the message body, since
the availability of a user-specified topic precludes us from
including it in the topic itself (which was the default behaviour).
This was technically a bug. For events that aren't unsupported
intentionally, the control should fall to the line that raises
UnknownWebhookEventType, and shouldn't be handled by anything else.
The events that are intentionally unsupported should be handled
more explicitly.
This improves test coverage for a lot of our webhooks that relied
on ad-hoc methods to handle unexpected event types.
Note that I have deliberately skipped github_legacy, it isn't
advertised and is officially deprecated.
Also, I have refrained from making further changes to Trello, I
believe further improvements to test coverage should be covered
in separate per-webhook commits/PRs.
We now use the name of the author of a commit as opposed to the
committer to format Git messages with multiple committers.
Part of #3968.
Follow-up to #4006.
We now show a few new things:
(1) The number of commits pushed.
(2) Who authored the commits (just counts, not which specific ones, for brevity).
Add tests for case of multiple committers.
Part of #3968.
Anders Kaseorg
Eeshan Garg
Steve Howell
Aditya Bansal
rht
Rhea Parekh
Tim Abbott
Yago González
umkay
Siddharth Mahapatra
Raghav Jajodia
Rishi Gupta
Tomasz Kolek