Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Go GitHub update #186

Closed
wants to merge 10 commits into from
Closed

Go GitHub update #186

wants to merge 10 commits into from

Conversation

mattburgess
Copy link

This bumps the vendored go-github library up to v21.0.1 in order to address https://github.com/terraform-providers/terraform-provider-github/issues/185. If doesn't quite get us up to the latest version due to google/go-github#1114.

I've committed each version bump separately in order to ease any bisection that might be necessary. This passed a make build && make test run but I'm unable to perform any acceptance tests.

@radeksimko
Copy link
Contributor

Hi @mattburgess
sorry it took us a while to get to your PR.

We have just recently migrated over to go modules which means this PR would need to be reworked quite significantly.

Additionally I think we should phase out the webhook name in a separate PR and address this in the schema too. I will raise a PR for that first.

@radeksimko
Copy link
Contributor

Also I just found this thread: google/go-github#1117

I'm not sure whether we want to phase out the field then. I will try to contact GitHub then.

@mattburgess
Copy link
Author

@radeksimko thanks for the feedback. I saw that PR before I started this one, but thought I'd raise it anyway based on not knowing when either might get merged. So, I'm happy for this one to be closed. I can try to help out by raising another version bump based on the new go modules way of doing things, or if you think it's trivial enough that me doing that just creates more work on your side then I'm happy to let it slide.

As it is, GitHub Enterprise 2.16.2 fixes the regression (https://enterprise.github.com/releases/2.16.2/notes) so selfishly I'm now no longer affected by the particular issue that drove this PR in the first place. More than happy to help out though if you think it'd be useful.

@bramwelt
Copy link

I just saw this PR and realized I was duplicating work in #203; Sorry! Happy to work on merging these two approaches (bump individual versions definitely helps for reviewing 👍 )

Hoping to get this updated so I can begin work on #87 . @radeksimko Any guidance would be appreciated.

@joestump
Copy link

joestump commented Apr 1, 2019

@bramwelt @radeksimko also interested in getting #87 across the finish line. Let me know if I can do anything to help out.

@paultyng
Copy link
Contributor

paultyng commented May 2, 2019

2.0.0 has been released, including SDK v25.0.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants