Support auto_merge field in PullRequest struct

[fho]

The pulls API returns an auto_merge field which is currently not available in the PullRequest struct.

When auto-merge is enabled for a Pull-Request the returned auto_merge field looks like:

  "auto_merge": {
    "enabled_by": {
      "login": "fho",
      "id": 514535,
      "node_id": "MDQ6VXNlcjUxNDUzNQ==",
      "avatar_url": "https://avatars.githubusercontent.com/u/514535?v=4",
      "gravatar_id": "",
      "url": "https://api.github.com/users/fho",
      "html_url": "https://github.com/fho",
      "followers_url": "https://api.github.com/users/fho/followers",
      "following_url": "https://api.github.com/users/fho/following{/other_user}",
      "gists_url": "https://api.github.com/users/fho/gists{/gist_id}",
      "starred_url": "https://api.github.com/users/fho/starred{/owner}{/repo}",
      "subscriptions_url": "https://api.github.com/users/fho/subscriptions",
      "organizations_url": "https://api.github.com/users/fho/orgs",
      "repos_url": "https://api.github.com/users/fho/repos",
      "events_url": "https://api.github.com/users/fho/events{/privacy}",
      "received_events_url": "https://api.github.com/users/fho/received_events",
      "type": "User",
      "site_admin": false
    },
    "merge_method": "rebase",
    "commit_title": "",
    "commit_message": ""
  },

It would be great if go-github could parse this field and make it accessible.

[gmlewis]

Thank you, @fho !

This would be a great PR for any new contributor to this repo or a new Go developer.
All contributions are greatly appreciated!

Feel free to volunteer for any issue and the issue can be assigned to you so that others don't attempt to duplicate the work.

Please check out our CONTRIBUTING.md guide to get started. (In particular, please remember to go generate ./... and don't use force-push to your PRs.)

Thank you!

[slayer321]

Hey @gmlewis, I am new to opensource and Golang, I will like to contribute to this project can you guide me on what needs to be done.
I have read the CONTRIBUTING.md and I also signed the CLA.

[gmlewis]

Hi @slayer321 - welcome to Go and to open source and thank you for your interest in helping out this project!

Sure... when attacking any kind of problem, it is best to break it down into smaller, more easily understandable parts.

So for this specific task, I would basically do the following:

• Fork and clone this repo if you haven't done so already
• Make sure you can run tests locally before you attempt to make any changes

As an aside, I personally use the following test script so that I don't forget anything:

#!/bin/bash -ex
go generate ./...
go test ./...
go vet ./...

• Create a new branch for your development... I like to put the issue in the name... something like:

$ git checkout -b i1881-support-auto-merge

OK, I can see this message is going to get huge... so I'll just start commenting and keep creating more comments as I break down the problem. Stay tuned... 😂

[gmlewis]

• Next, since @fho was so kind as to provide us a URL regarding the issue, I take a look at it.
• I see that the endpoint in question is: GET /repos/{owner}/{repo}/pulls
• I now try to find out where this is in our repo's code base by searching for repos/%v/%v/pulls and making sure it is a GET method

OK, I'm getting pulled into a meeting so I'm going to have to continue to address this later today.

[gmlewis]

• I then match that endpoint with the code, in this case: https://github.com/google/go-github/blob/master/github/pulls.go#L135-L145 and check that the URL for the method is pointing to the same endpoint.
• I see that PullRequest is the struct being returned, which is located here: https://github.com/google/go-github/blob/master/github/pulls.go#L21-L77
• I then search for the "auto_merge" field which is what is currently missing... both in the docs and in the struct... and yes, I confirm that it is in the docs ("auto_merge": null) and not in the code.
• I see that the author of the issue shows an example response for this field even though the GitHub v3 API documentation does not, so I base my implementation on the provided example.
• It looks to me like the "auto_merge" field has 4 sub-fields and should go at the top-level of the PullRequest struct... and one of the fields looks very much like our existing User struct. So I would write some code like this:

// PullRequestAutoMerge represents the "auto_merge" response for a PullRequest.
type PullRequestAutoMerge struct {
    EnabledBy *User `json:"enabled_by,omitempty"`
    MergeMethod *string `json:"merge_method,omitempty"`
    CommitTitle *string `json:"commit_title,omitempty"`
    CommitMessage *string `json:"commit_message,omitempty"`
}

Then I would add this new struct into the PullRequest struct, probably after RequestedReviewers:

...
	RequestedReviewers  []*User    `json:"requested_reviewers,omitempty"`
	AutoMerge *PullRequestAutoMerge `json:"auto_merge,omitempty"`
...

Afterward, I would ideally incorporate this new value in an existing or new unit test (following examples in the surrounding code)... although it is not always necessary to do this just when adding new fields. It is a nice-to-have to make sure that the struct gets populated properly from JSON... and you could use the example given by the author of the issue in this case.

Finally, I would test out all my changes using that same script I shared above and make sure everything is working properly, then I would share my PR on GitHub and ask for a code review.

I think that about summarizes things... please let me know if you have any questions, and best wishes to you.

[fho]

Wow, I did not expect such a fast response. :-)
@slayer321 thanks a lot for volunteering! Please let me know if you should change your plans to do the implementation, then I would pick it up instead
@gmlewis excellent step-by-step guide! :-)

[slayer321]

Thank you very much @gmlewis for explaining everything step by step. Most of the steps I understood and some I didn't but I am learning about it.

As you have assigned it to me I will start working on it and if I face any problems I will let you know 😃

[bharathk005]

@gmlewis Since there were no updates for around 11 days, I thought I will go ahead and give this a try. I have created a PR #1894 for the same. Please check and let me know.
Also, I feel it would be nice to have all the test files in a separate folder

[gmlewis]

Sorry, @bharathk005 , but let us give @slayer321 an opportunity to respond before taking over an issue that is already assigned.

Additionally, I looked at your PR and it has a number of problems, so I'm not going to officially review it until we hear back from @slayer321.

As for putting test files in a separate folder, that is not idiomatic Go and we will not accept that kind of change in this repo.

[bharathk005]

@gmlewis
Thanks for the quick response. Sure let's wait for @slayer321

[slayer321]

Sorry, @gmlewis for replying so late. I was not able to work on this issue as my college exam started and I was busy with that but now as the exams are over I can start working on it.

[slayer321]

@gmlewis Before opening a PR I have some doubt that I think needs to be clear.
sorry if this seems naive as this is a first time I am working with golang and testing.

I added all the code that needs to be added but I am bit confuse about the testing part.

Afterward, I would ideally incorporate this new value in an existing or new unit test (following examples in the surrounding code)... although it is not always necessary to do this just when adding new fields. It is a nice-to-have to make sure that the struct gets populated properly from JSON... and you could use the example given by the author of the issue in this case.

As you have mentioned above that after adding the fields we need to test the code so I found a file name fields.go in test/fields dir and then I set the GITHUB_TOKEN env var
and when I run the fields.go command this is what I am getting

*github.User missing field for key: hireable (example value: <nil>)
*github.User missing field for key: twitter_username (example value: <nil>)
*github.User missing field for key: hireable (example value: <nil>)
*github.User missing field for key: company (example value: <nil>)
*github.User missing field for key: email (example value: <nil>)
*github.Organization missing field for key: company (example value: <nil>)
*github.Organization missing field for key: blog (example value: <nil>)
*github.Organization missing field for key: name (example value: <nil>)
*github.Organization missing field for key: email (example value: <nil>)
*github.Organization missing field for key: twitter_username (example value: <nil>)
*github.Organization missing field for key: location (example value: <nil>)
*github.Repository missing field for key: temp_clone_token (example value: )
*github.Repository missing field for key: forks (example value: 1533)
*github.Repository missing field for key: mirror_url (example value: <nil>)
*github.Repository missing field for key: watchers (example value: 7537)
*github.Repository missing field for key: open_issues (example value: 69)
*github.Issue missing field for key: milestone (example value: <nil>)
*github.Issue missing field for key: active_lock_reason (example value: <nil>)
*github.Issue missing field for key: performed_via_github_app (example value: <nil>)
error: GET https://api.github.com/gists/9257657: 502 Server Error []  

seeing this error I think that I need to give the github username is that what's needed?
and if yes where should I add it .

apart from all this I run all the 3 test command and getting no errors

go generate ./...
go test ./...
go vet ./...

[gmlewis]

seeing this error I think that I need to give the github username is that what's needed?
and if yes where should I add it .

Sorry, @slayer321, but it is really difficult to diagnose and troubleshoot problems without seeing the code and seeing the exact command that you are using to run the code.

Can you please either create the PR or a GitHub gist that shows what you are trying to do and how you are calling it?

Make sure to NOT including any personal access tokens in your post, as you would have to immediately delete them if you do.

Thanks.

[gmlewis]

Hi @slayer321 - #1896 LGTM... nice job!
It would be great if you could test it out on an actual repo (like the test case given above) to make sure it works.

It sounds like you were having challenges with that testing... feel free to post a GitHub gist (without your personal access token) and I'll see if I can figure out what is going wrong with it.

[slayer321]

Hii @gmlewis , I did the test as I was mention in test dir and for output I am adding this Github gist : https://gist.github.com/slayer321/b2b1dd1f68ce3a83732731f3177b7874

[gmlewis]

As I commented on the gist, you can ignore the existing integration test.

Please attempt to write your own stand-alone test to make sure that the new code works. You don't need to check it in to the repo, but just make sure that it does what you expect.

Thank you, @slayer321 !

[fho]

Thanks a lot for the implementation @slayer321.
I started to make use of the change and realized that the PullRequestEvent struct is also missing the AutoMerge field.
The Automerge structure is exactly the same in PullRequestEevent. I checked a github webhook event that I received.

I don't know how you want to handle it, @gmlewis.
Should I create a separate new github issue for that?
Or could we include it in #1896 already instead?

[gmlewis]

I don't know how you want to handle it, @gmlewis.
Should I create a separate new github issue for that?
Or could we include it in #1896 already instead?

Since #1896 is not yet merged, I'm fine if @slayer321 would like to add it to that PR.

[slayer321]

Hi @gmlewis, yeah I will like to add changes that need to be done.
Before adding anything just wanted to confirm that I interpreted things right whatever was said by @fho

I started to make use of the change and realized that the PullRequestEvent struct is also missing the AutoMerge field.
The Automerge structure is exactly the same in PullRequestEevent. I checked a Github webhook event that I received.

I found the PullRequestEvent struct in event_types.go file in github dir
so the changes that need to be done is to add the AutoMerge field in PullRequestEvent struct something like this

...
Before      *string         `json:"before,omitempty"`
After        *string          `json:"after,omitempty"`
AutoMerge     *PullRequestAutoMerge   `json:"auto_merge,omitempty"`
...

is this correct?

[gmlewis]

is this correct?

That sounds right to me. @fho - can you please confirm?

[fho]

@slayer321
Yes, that is correct. That is all.

[fho]

Sorry, for the confusion. Please forget what I said. :-)

The Automerge field must not be added to the PullRequest struct.
In the PullRequestEvent the auto_merge field is nested in the PullRequest struct.

[slayer321]

oh! thank you for letting us know. :)
so I will not open another PR.

[simongottschlag]

#1896 is now merged 😁🎉 Thank you @slayer321!

@gmlewis when can a release be expected? 😊👍

[gmlewis]

@gmlewis when can a release be expected?

Yes indeed, @simongottschlag , but I've got a lot more merges to perform first. Hopefully by later tonight you will have a new release.

[simongottschlag]

@gmlewis that's awesome! Sorry if it read like "I expect a release now!" - it was more a question like "How is the usual release cadence? I'm in no hurry, but really looking forward to it!".

Thank you for the hard work! 🥇

[gmlewis]

Fixed by #1896.

[gmlewis]

Sorry if it read like "I expect a release now!" - it was more a question like "How is the usual release cadence? I'm in no hurry, but really looking forward to it!".

No problem at all, @simongottschlag! I was just trying to give an accurate estimate. 😄

This should now be available in Release v37.0.0: https://github.com/google/go-github/releases/tag/v37.0.0