golang/go

x/build/maintner: GitHubIssue.ClosedBy field is never populated

Open

#28,745 opened on Nov 12, 2018

View on GitHub
 (4 comments) (0 reactions) (0 assignees)Go (19,008 forks)batch import
BuildersNeedsFixhelp wanted

Repository metrics

Stars
 (133,883 stars)
PR merge metrics
 (No merged PRs in 30d)

Description

There exists a GitHubIssue.ClosedBy field in maintner:

type GitHubIssue struct {
    ...
    ClosedBy *GitHubUser
    ...
}

What did you expect to see?

Accurate values.

What did you see instead?

The field is never populated and always equals to nil for all GitHub issues.

This can be misleading for anyone looking to use that information.

Cause

The closed_by JSON field is documented and shown in the sample response at https://developer.github.com/v3/issues/#get-a-single-issue.

However, maintner uses the https://developer.github.com/v3/issues/#list-issues-for-a-repository endpoint for getting information about many issues at once:

https://github.com/golang/build/blob/23803abc1638efbf100d69fe6d901b14a9ad55fd/maintner/github.go#L1605-L1613

But GitHub doesn't include all detailed fields when listing many issues rather than getting a single issue. The closed_by field is indeed missing:

  ...
  "comments": 1,
  "created_at": "2018-11-09T00:20:31Z",
  "updated_at": "2018-11-09T00:25:34Z",
  "closed_at": "2018-11-09T00:22:12Z",
  "author_association": "MEMBER",
  "body": "It exists in [...]",
  "closed_by": {
    "login": "gopherbot",
    ...
  }
}
    ...
    "comments": 1,
    "created_at": "2018-11-09T00:20:31Z",
    "updated_at": "2018-11-09T00:25:34Z",
    "closed_at": "2018-11-09T00:22:12Z",
    "author_association": "MEMBER",
    "body": "It exists in [...]"
  },

Possible Fixes

I see two possible solutions:

  1. Populate the field.
  2. Remove the field (or document it as broken and point to this issue).

Since this field isn't included in the existing endpoint queried by maintner, it would require making additional API calls per issue. That can be extremely expensive and simply not viable.

I'd suggest removing it or documenting, at least as an intermediate step. But open to ideas. /cc @bradfitz

Contributor guide