search

octokit / go-octokit

Simple Go wrapper for the GitHub API
https://github.com/octokit/go-octokit
MIT License
258 stars 79 forks source link

Implement '/meta' #118

Closed pcasaretto closed 8 years ago

pcasaretto commented 9 years ago

Hey guys,

Learning go and currently using go-octokit as a dependency for another project. I wanted to contribute back and also scratch my own itch :) . Opening this PR with WIP to get early feedback. The code currently works.

Some specific questions on the code itself.

Thanks!

brodock commented 8 years ago

@pengwynn can you please provide some feedback here? We are implementing other missing pieces and would like to know if you are interested in more PRs.

pengwynn commented 8 years ago

@pcasaretto @brodock: My apologies, I was on vacation when this landed. Was buried in my :star: folder.

I left some feedback. Thanks for the patch!

pcasaretto commented 8 years ago

Thanks for the feedback @pengwynn. Tried to incorporate all of it in the most recent commit. Apart from the obvious error cases, should I focus on any specific test cases?

pcasaretto commented 8 years ago

I've added tests for the basic cases, but there is lack of coverage where the IPs and IPNets get parsed. Do you think it's important to test that? I did try but I could't stub the request on the same test and had to add two different json fixtures. I figured it wasn't worth the trouble.

pengwynn commented 8 years ago

I've added tests for the basic cases, but there is lack of coverage where the IPs and IPNets get parsed. Do you think it's important to test that?

I think this is fine for now. Thanks for the patch. :cake: