golang/go

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

Ouverte

#28 745 ouverte le 12 nov. 2018

 (4 commentaires) (0 réaction) (0 personne assignée)Go (19 008 forks)batch import
BuildersNeedsFixhelp wanted

Métriques du dépôt

Stars
 (133 883 étoiles)
Métriques de merge PR
 (Aucune PR mergée en 30 j)

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

Guide contributeur