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

Adding BaseRequestBuilder.SetURL #9

Merged

Conversation

@DeepDiver1975
Copy link
Contributor

DeepDiver1975 commented Feb 20, 2020

This allows to specify an alternative BaseURL for the client.

gen/msgraph.go.tmpl Outdated Show resolved Hide resolved
@yaegashi

This comment has been minimized.

Copy link
Owner

yaegashi commented Feb 21, 2020

@DeepDiver1975 Thanks for the PR!

I used to think of this kind of feature to enable custom baseURL. An alternative would be adding a constructor method with another signature like NewClientWithBaseURL(cli *http.Client, baseURL string) and keeping baseURL immutable after creation. Which is better?

Could you share some about the actual use cases you expect? Part of the unit testing infrastructure?

@DeepDiver1975

This comment has been minimized.

Copy link
Contributor Author

DeepDiver1975 commented Feb 21, 2020

Could you share some about the actual use cases you expect?

There are open source projects out there which implement the graph api and in this case the graph is accessible on different domains.

I'll update the PR asap. 👍

@DeepDiver1975 DeepDiver1975 force-pushed the DeepDiver1975:feature/BaseRequestBuilder.SetURL branch from 37648f5 to c32678b Feb 21, 2020
@DeepDiver1975 DeepDiver1975 requested a review from yaegashi Feb 21, 2020
@yaegashi

This comment has been minimized.

Copy link
Owner

yaegashi commented Feb 21, 2020

Ok, after thiking a bit, I decided not to insist on the immutability of baseURL, so only adding SetURL() would suffice. I will merge the PR after amending it.

There are open source projects out there which implement the graph api and in this case the graph is accessible on different domains.

Nice to hear that msgraph.go has been somehow useful for projects by others. 😄

@yaegashi yaegashi merged commit 2d438cf into yaegashi:master Feb 21, 2020
@DeepDiver1975

This comment has been minimized.

Copy link
Contributor Author

DeepDiver1975 commented Feb 21, 2020

Nice to hear that msgraph.go has been somehow useful for projects by others. smile

Oh - absolutly useful! THX a lot for it!

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

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.