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

Conversation

@jklausa
Copy link
Contributor

@jklausa jklausa commented Oct 11, 2018

This his mostly contents of #25, which was mostly reviewed already by me and @ScoutHarris before — just with some minor cleanups and with latest develop merged in.

I'm gonna leave it open after the review for a while, since it's possible I might need to do some changes while working on the WPiOS part of it, but it's close enough where I thought another pair of eyes wouldn't hurt :)

"only_wordpressdotcom": true as AnyObject]
}
return result
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Could every case return directly the [String: AnyObject]? Also could the function be private?

throw DecodingError.dataCorruptedError(
in: container,
debugDescription: "Cannot decode date string \(dateStr)"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the if let be guard let date ... and throw the error? So we can return the date after it without the if else statement.

@danielebogo
Copy link
Contributor

Hey @jklausa I only left 2 comments, but it looks great so far!

@jklausa jklausa closed this Oct 31, 2018
@jklausa jklausa deleted the feature/custom-domains branch October 31, 2018 03:21
@jklausa
Copy link
Contributor Author

jklausa commented Oct 31, 2018

uhh not what i was going for exactly here

@jklausa jklausa reopened this Oct 31, 2018
@jklausa jklausa restored the feature/custom-domains branch October 31, 2018 03:27
@aerych
Copy link
Contributor

aerych commented Nov 7, 2018

Hey @jklausa
I have nothing to add over @danielebogo's comments. Once those are in and the conflicts resolved I think its good to go.

@jklausa
Copy link
Contributor Author

jklausa commented Nov 8, 2018

@aerych GitHub is having some weird hiccups lately — I've addressed those way back when, but it's not showing in the inline diffs — you have to go to "Files changed" up top to see it :)

@jklausa
Copy link
Contributor Author

jklausa commented Nov 9, 2018

@aerych can i consider your thumbs up reactji above as :shipit: :)?

Sent with GitHawk

@aerych
Copy link
Contributor

aerych commented Nov 9, 2018

Fo sho! :shipit: with much 🚢 ing

@jklausa jklausa merged commit b7b4ad2 into develop Nov 9, 2018
@jklausa jklausa deleted the feature/custom-domains branch February 19, 2019 13:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants