Custom Url configuration #1128
Merged
Custom Url configuration #1128
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jamesmckenzie
changed the title
ob-stripe
suggested changes
Mar 12, 2018
| { | ||
| if (string.IsNullOrEmpty(_apiBase)) | ||
| { | ||
| _apiBase = "https://api.stripe.com/v1"; |
There was a problem hiding this comment.
Rather than hardcoding the default value here, could you instead use a constant? E.g. Urls.DefaultBaseUrl.
| { | ||
| if (string.IsNullOrEmpty(_uploadsBase)) | ||
| { | ||
| _uploadsBase = "https://uploads.stripe.com/v1"; |
| { | ||
| if (string.IsNullOrEmpty(_connectBase)) | ||
| { | ||
| _connectBase = "https://connect.stripe.com"; |
| @@ -14,7 +14,7 @@ internal static class Urls | |||
|
|
|||
| public static string Coupons => BaseUrl + "/coupons"; | |||
|
|
|||
| public static string Plans => BaseUrl + "/plans"; | |||
| public static string Plans => BaseUrl + "/plans"; | |||
There was a problem hiding this comment.
You have some unnecessary whitespace here.
|
Hi @jamesmckenzie, thanks a lot for the PR! Overall this looks fairly solid to me. I left a few comments, could you take a look? |
|
Thanks for taking a look. I've made the requested changes - let me know if there's anything else |
|
and mistake fixed! |
ob-stripe
approved these changes
Mar 12, 2018
|
Thanks @jamesmckenzie! This looks good to me. Can I bother you to squash the 3 commits into a single one before I pull this in? |
jamesmckenzie
force-pushed
the
add-custom-base-urls
branch
from
a850325 to
59ec275
Compare
|
not a problem, done! |
|
Perfect! Pulling this in. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1102
allows consumers of the library to easily override the base urls for the stripe api to allow for easier testing (for example, with the stripe-mock service)