pullRequestReview.review.htmlUrl - #946
Conversation
Sorry, something went wrong.
| private String commit_id; | ||
| private GHPullRequestReviewState state; | ||
| private String submitted_at; | ||
| private String html_url; |
There was a problem hiding this comment.
The ObjectMapper configuration explicitly declares snake_case strategy, but there is no single standard of field naming - I see both standard java (htmlUrl) and snake case (html_url) over the project (only fields, not getters for sure). This is confusing, IMHO it should be better to standardize it (personal preference - to java).
Sorry, something went wrong.
There was a problem hiding this comment.
@seregamorph "standardize to java naming" - agreed. It just hasn't been a priority. Especially since we don't have
sufficient code coverage to have high confidence in tests ensuring it is done right. PR welcome.
Sorry, something went wrong.
| @Override | ||
| public URL getHtmlUrl() { | ||
| return null; | ||
| return GitHubClient.parseURL(html_url); |
There was a problem hiding this comment.
back compatibility saved. If field is not present, the return value is still null
Sorry, something went wrong.
|
Please note, that https://docs.github.com/en/rest/reference/pulls#reviews does not explicitly declare this parameter. I assume it only present in the webhook. |
Sorry, something went wrong.
Description
Webhook event GHPullRequestReviewEvent: pull_request_review has review object in it which declares html_url field:
{ "action": "submitted", "review": { "id": 496232606, "node_id": "MDE3OlB1bGxSZXF1ZXN0UmV2aWV3NDk2MjMyNjA2", "user": { "login": "schernov-xo", "id": 41187778, "node_id": "MDQ6VXNlcjQxMTg3Nzc4", "avatar_url": "https://avatars3.githubusercontent.com/u/41187778?v=4", "gravatar_id": "", "url": "https://api.github.com/users/schernov-xo", "html_url": "https://github.com/schernov-xo", "followers_url": "https://api.github.com/users/schernov-xo/followers", "following_url": "https://api.github.com/users/schernov-xo/following{/other_user}", "gists_url": "https://api.github.com/users/schernov-xo/gists{/gist_id}", "starred_url": "https://api.github.com/users/schernov-xo/starred{/owner}{/repo}", "subscriptions_url": "https://api.github.com/users/schernov-xo/subscriptions", "organizations_url": "https://api.github.com/users/schernov-xo/orgs", "repos_url": "https://api.github.com/users/schernov-xo/repos", "events_url": "https://api.github.com/users/schernov-xo/events{/privacy}", "received_events_url": "https://api.github.com/users/schernov-xo/received_events", "type": "User", "site_admin": false }, "body": "", "commit_id": "c3acd4733a645e1c6ae3b6da83068ae12099f616", "submitted_at": "2020-09-25T09:52:57Z", "state": "commented", "html_url": "https://github.com/trilogy-group/northplains-telescope-api/pull/21#pullrequestreview-496232606", "pull_request_url": "https://api.github.com/repos/trilogy-group/northplains-telescope-api/pulls/21", ...Old implementation ignores it (always null).
Proof: see the test, event existing test payload pull_request_review.json has it (ignored in test).
Before submitting a PR:
We love getting PRs, but we hate asking people for the same basic changes every time.
When creating a PR: