golang/go

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

Aperta

#28.745 aperta il 12 nov 2018

 (4 commenti) (0 reazioni) (0 assegnatari)Go (19.008 fork)batch import
BuildersNeedsFixhelp wanted

Metriche repository

Star
 (133.883 stelle)
Metriche merge PR
 (Nessuna PR mergiata in 30 g)

Descrizione

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

Guida contributor