search

nicklaw5 / helix

A Twitch Helix API client written in Go.
MIT License
243 stars 88 forks source link

Added APIBaseURL option to the Options struct #39

Closed uranoxyd closed 4 years ago

uranoxyd commented 4 years ago

Hope everything is fine, this is my first PR :)

Closes #38

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 148

Totals Coverage Status
Change from base Build 144: 0.03%
Covered Lines: 628
Relevant Lines: 699
💛 - Coveralls
nicklaw5 commented 4 years ago

Congratz on your first PR, may there be many more :)

Can I ask why you'd like the ability to change the base URL? What are you trying to achieve?

uranoxyd commented 4 years ago

Thanks :) I am writing a kind of relay with load balancer and caching functionality. And i like to use your library with it, so your library has to connect to my enpoint instead of directly to the Twitch endpoint.

uranoxyd commented 4 years ago

As for the test, I am unfortunately extremely inexperienced in writing tests. Even if I knew how to implement the test now, I wouldn't know what a test for it should look like. Or what should be tested.

nicklaw5 commented 4 years ago

As for the test, I am unfortunately extremely inexperienced in writing tests. Even if I knew how to implement the test now, I wouldn't know what a test for it should look like. Or what should be tested.

No troubles. I can add some tests after we merge this change.

uranoxyd commented 4 years ago

Now the question is, where do we go from here? I am not familiar with the github features at all. Should I press the Resolve conversation buttons? :)

Edit: And in case, regarding the APIBaseURL to DefaultAPIBaseURL rename, could i do this now and "add" it to this PR or would this be another PR?

nicklaw5 commented 4 years ago

Now the question is, where do we go from here? I am not familiar with the github features at all. Should I press the Resolve conversation buttons? :)

Typically, you would create one or more new commits that resolve the comments I've added to the code. Once you push the commit changes to GitHub, those conversations will automatically be resolved.

nicklaw5 commented 4 years ago

Looks good :+1:

You'll notice the comments I added are now marked as Outdated. That essentially means they've been resolved.

nicklaw5 commented 4 years ago

If you're done making changes, I'm happy to merge.

uranoxyd commented 4 years ago

looks good. And thanks for your patience :)