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

Fixes #320. Fixes compiler errors and warnings encountered with Xcode 9.3 beta 2 #322

Merged

Conversation

rdingman
Copy link
Contributor

@rdingman rdingman commented Feb 8, 2018

No description provided.

@rdingman rdingman force-pushed the bugfix/xcode-9.3-beta-2-fixes branch from 453954b to 6aab93d Compare February 8, 2018 06:56
@ZevEisenberg
Copy link
Collaborator

@rdingman thanks for this PR! I've got one little tweak that I'd love to see:

I prefer to avoid having Swift-version-specific conditionals in the main code wherever possible. Instead, I like to back-port new functions to the old version of Swift. We wrote up a blog post about this, but the short version is that, in the existing Compatibility.swift, we would expose a function called compactMap only on Swift 4.0. This way, we don't have two branches in StringStyle.swift that we have to keep in sync with each other.

How does that sound? Please let me know if you'd like clarification, or if you don't have time to make the update, I can take it from here.

@rdingman
Copy link
Contributor Author

rdingman commented Feb 8, 2018

Makes sense. I'll try to make those changes today.

@ZevEisenberg
Copy link
Collaborator

@rdingman perfect. Going to wait a little to merge until the Xcode is a little further along, because future betas may break more things, but I may publish this as a release candidate soon.

@ZevEisenberg ZevEisenberg merged commit eb8fd63 into Rightpoint:master Feb 11, 2018
@ZevEisenberg
Copy link
Collaborator

This has been released as part of BonMot 5.1. Thanks for your contribution, @rdingman! 🚀

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

Successfully merging this pull request may close these issues.

2 participants