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

Conversation

@ScoutHarris
Copy link
Contributor

@ScoutHarris ScoutHarris commented Aug 20, 2019

Related WPKit PR: wordpress-mobile/WordPressKit-iOS#181

This adds methods to create a WordPress account by sending Apple ID credentials to the method added in the above WPKit PR.

This can be tested with WPiOS PR: wordpress-mobile/WordPress-iOS#12350

@ScoutHarris ScoutHarris requested a review from frosty August 21, 2019 01:14
@ScoutHarris
Copy link
Contributor Author

Hey @frosty . I realize the checks are failing. I've asked @jkmassel for assistance (unless you can identify the problem). But the WPiOS branch should build fine, enabling testing. At the very least, the podspec will be corrected once all the pods are official.

Copy link
Contributor

@frosty frosty left a comment

Choose a reason for hiding this comment

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

One small comment, otherwise looks great!

guard let name = components else {
return ""
}
return PersonNameComponentsFormatter().string(from: name)
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 really know PersonNameComponentsFormatter but I wonder if we should use the static localizedString method and specify the style of name

PersonNameComponentsFormatter.localizedString(from: components,
                                              style: .medium,
                                              options: [])

🤔

Copy link
Contributor Author

@ScoutHarris ScoutHarris Aug 21, 2019

Choose a reason for hiding this comment

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

The default style is medium, so specifying that shouldn't be necessary. But, ya, I should have used localizedString. Thanks. I'll change it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, reading the documentation again, I don't think a change is necessary. string seems to return the name in the correct style for the language. localizedString is for specifying the format and options. Which we don't need to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, great! Thanks for reading the docs 😅

@ScoutHarris ScoutHarris merged commit d0f6bda into develop Aug 21, 2019
@ScoutHarris ScoutHarris deleted the feature/siwa_create_account branch August 21, 2019 18:55
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