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

fix api preview header, missing : #1786

Closed
wants to merge 0 commits into from

Conversation

jurgenweber
Copy link

so then the api previews actually work. :)

@google-cla
Copy link

google-cla bot commented Jan 27, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Jan 27, 2021
@jurgenweber
Copy link
Author

@googlebot I signed it!

@jurgen-weber-deltatre
Copy link

jurgen-weber-deltatre commented Jan 28, 2021

considering all the failing CI, this doesn't seem to be the only place/thing I need to update but it is also making me second guess that this is the solution...

but for me, this works:

$ curl -v -XPOST -H "Accept: application/vnd.github.nebula-preview+json, application/vnd.github.v3+json" -H "Authorization: token XXXX" "https://api.github.com/orgs/myorg/repos" -d '{"name":"jurgen","visibility":"internal"}'

This does not:

$ curl -v -XPOST -H "Accept application/vnd.github.nebula-preview+json, application/vnd.github.v3+json" -H "Authorization: token XXXX" "https://api.github.com/orgs/myorg/repos" -d '{"name":"jurgen","visibility":"internal"}'

but it would mean these api previews are working for no one.

@gmlewis
Copy link
Collaborator

gmlewis commented Jan 28, 2021

so then the api previews actually work. :)

I'm sorry, I don't understand why this change is necessary. This repo has been using custom preview headers for years now and they have been working well up to this point. Could you please explain what you saw that caused you to write this PR?

Also, you should be able to run "go test ./..." locally to make sure the unit tests pass.

@jurgen-weber-deltatre
Copy link

I'm sorry, I don't understand why this change is necessary. This repo has been using custom preview headers for years now and they have been working well up to this point.

Yeha, I agree.. Seems weird to me.

Could you please explain what you saw that caused you to write this PR?

#1786 (comment)

It was a long story to get to this point, but I feel there is some more to it. I started with using the terraform-provider-github and in the end I can not make internal repos using the provider. I have subsequently ended up here. I started replicating the provider commands with curl commands to ID what is going on.

The beginings of my journey are here; integrations/terraform-provider-github#662

If it isn't the headers, then I am not sure to why when using TF this fails.

GitHub support then pointed me to the concept of the API reviews and I noticed the string difference, using CURL the api call fails without a colon.

Also, you should be able to run "go test ./..." locally to make sure the unit tests pass.

Ok

@jurgen-weber-deltatre
Copy link

Just a follow up; I tried this locally (found some time this morning):

https://github.com/jurgenweber/go-github/blob/master/example/orgnewrepo/main.go

this works, so the problem is else wehre.

Thanks

@jurgenweber
Copy link
Author

I found it; integrations/terraform-provider-github#680

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Indication that the PR author has signed a Google Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants