Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@jtreanor
Copy link
Contributor

As I have been developing networking mocking, I have discovered a few places that the WP.com API URL needs to be injected which I missed in #93.

This PR makes sure every instance of WordPressComRestApi in WordPressAutheticator uses the injectable wpcomAPIBaseURL.

@jtreanor jtreanor requested a review from jkmassel May 14, 2019 15:44
Copy link
Contributor

@jkmassel jkmassel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one optional suggestion, but apart from that:

✅ Code compiles
✅ Tests pass
✅ Code changes look sensible
:shipit:

private func checkEmailAvailability(completion:@escaping (Bool) -> ()) {

let remote = AccountServiceRemoteREST(wordPressComRestApi: WordPressComRestApi())
let remote = AccountServiceRemoteREST(wordPressComRestApi: WordPressComRestApi(baseUrlString: WordPressAuthenticator.shared.configuration.wpcomAPIBaseURL))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we enforce it on this repo, but this line is rather long – could probably be broken up into smaller statements to be a bit more understandable?

Up to you :)

@jtreanor jtreanor merged commit 615b0c2 into develop May 17, 2019
@jtreanor jtreanor deleted the upate-rest-api-usage branch May 17, 2019 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants