Go-github: Search Commits API Results Problem

[amirilovic]

Issue: When calling search commits api (go-github/github/search.go:84) returned results contain correct number of items but each item properties are empty.

As per github search docs https://developer.github.com/v3/search/#search-commits

Search Commits is returning results as:

{
  "total_count": 1,
  "incomplete_results": false,
  "items": [
    {
      "url": "https://api.github.com/repos/octocat/Spoon-Knife/commits/bb4cc8d3b2e14b3af5df699876dd4ff3acd00b7f",
      "sha": "bb4cc8d3b2e14b3af5df699876dd4ff3acd00b7f",
      "html_url": "https://github.com/octocat/Spoon-Knife/commit/bb4cc8d3b2e14b3af5df699876dd4ff3acd00b7f",
      "comments_url": "https://api.github.com/repos/octocat/Spoon-Knife/commits/bb4cc8d3b2e14b3af5df699876dd4ff3acd00b7f/comments",
      "commit": {
        "url": "https://api.github.com/repos/octocat/Spoon-Knife/git/commits/bb4cc8d3b2e14b3af5df699876dd4ff3acd00b7f",
        "author": {
          "date": "2014-02-04T14:38:36-08:00",
          "name": "The Octocat",
          "email": "[email protected]"
        },
        "committer": {
          "date": "2014-02-12T15:18:55-08:00",
          "name": "The Octocat",
          "email": "[email protected]"
        },
        "message": "Create styles.css and updated README",
        "tree": {
          "url": "https://api.github.com/repos/octocat/Spoon-Knife/git/trees/a639e96f9038797fba6e0469f94a4b0cc459fa68",
          "sha": "a639e96f9038797fba6e0469f94a4b0cc459fa68"
        },
        "comment_count": 8
      },
      "author": {
        // author fields ...
      },
      "committer": {
       // committer fields ...
      },
      "parents": [
        {
          "url": "https://api.github.com/repos/octocat/Spoon-Knife/commits/a30c19e3f13765a3b48829788bc1cb8b4e95cee4",
          "html_url": "https://github.com/octocat/Spoon-Knife/commit/a30c19e3f13765a3b48829788bc1cb8b4e95cee4",
          "sha": "a30c19e3f13765a3b48829788bc1cb8b4e95cee4"
        }
      ],
      "repository": {
       // repo fields ...
      },
      "score": 4.9971514
    }
  ]
}

But in search.go:67 results are deserialized in CommitResult struct which has different layout:

type CommitResult struct {
    Hash           *string     `json:"hash,omitempty"`
    Message        *string     `json:"message,omitempty"`
    AuthorID       *int        `json:"author_id,omitempty"`
    AuthorName     *string     `json:"author_name,omitempty"`
    AuthorEmail    *string     `json:"author_email,omitempty"`
    AuthorDate     *Timestamp  `json:"author_date,omitempty"`
    CommitterID    *int        `json:"committer_id,omitempty"`
    CommitterName  *string     `json:"committer_name,omitempty"`
    CommitterEmail *string     `json:"committer_email,omitempty"`
    CommitterDate  *Timestamp  `json:"committer_date,omitempty"`
    Repository     *Repository `json:"repository,omitempty"`
}

So, returned results are empty.

[dmitshur]

This issue seems valid. Any ideas how this happened?

It looks like CommitResult was added recently, just 2 months ago in #520. /cc @sahildua2305 @gmlewis It's a preview API, announced at https://developer.github.com/changes/2017-01-05-commit-search-api/. Has it changed since back then, or did we make a mistake in #520?

I just want to understand what happened better (in order to learn from it, and to be more confident this is a valid issue; not looking to place blame on anyone, making mistakes is okay, they can be fixed once spotted and understood).

I've looked through https://developer.github.com/changes/ and not spotting any changes. So my current best guess is that we made a mistake when reviewing #520 and this was never correct.

Or perhaps GitHub did change the preview API, but didn't announce that (or maybe their docs page had a mistake that they've since fixed).

The q search term docs seem to have fields that match the current CommitResult, perhaps that's what was used by mistake?

(From https://developer.github.com/v3/search/#search-commits.)

But it doesn't have them all, so perhaps not.

Any additional insight is welcome. Of course, it's not neccessary to figure this out, but I think it'd be helpful, if possible.

[amirilovic]

@shurcooL My reasoning exactly the same. I've tried to find if docs have changed in the meantime on https://web.archive.org/web/20170301000000*/https://developer.github.com/v3/search but there is now history for January 2017.

[aleksclark]

@gmlewis / @shurcooL I'd like to work on this if that's acceptable :) Also, is mentioning a maintainer needed generally? If so I'll submit a docs PR, if not I'll stop doing it

[sahildua2305]

Indeed, @shurcooL, it's weird to have this broken without any documented change from GitHub.

[amirilovic]

@aleksclark I've already submitted a pull request #602

[aleksclark]

@amirilovic facepalm of course I noticed that about 20 seconds before your comment. nevermind :P

[gmlewis]

Yes, very odd, but this looks like the correct way to proceed. Thanks, all, for catching and fixing this.

[gmlewis]

@shurcooL - should #602 have bumped the libraryVersion number?
I'm thinking now that it should have. Thoughts?

[dmitshur]

Yes, it should've. The Go API has changed. It's done to match the GitHub API, but it's still a breaking API change.

It's fine to do so in a followup commit. Maybe even bundle a few changes into one version bump.

[gmlewis]

OK, we have a few PRs in the queue. Let's get it bumped shortly. I won't create a new PR just for the bump.

[dmitshur]

Bumped it in 5b3f96e055a0e3585c2389cc5ee70a7099a07062.