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

Swift 3 API changes #19

Closed
wants to merge 1 commit into from
Closed

Conversation

phatblat
Copy link
Collaborator

@phatblat phatblat commented Jul 6, 2016

Targeting master as it's the only branch. Do you want this in a separate swift3 branch?

@AliSoftware
Copy link
Owner

👍 yes a dedicated swift3 branch seems more appropriate

@AliSoftware
Copy link
Owner

Pulled in https://github.com/AliSoftware/Reusable/tree/swift3 👍

Thx a lot. I've added you as collaborator for the occasion (so you can create branch directly on Reusable in the future instead of needing your own fork)

@AliSoftware AliSoftware closed this Jul 6, 2016
@phatblat phatblat deleted the swift3 branch July 6, 2016 20:37
@phatblat
Copy link
Collaborator Author

phatblat commented Jul 6, 2016

💃 Sweet!

@AliSoftware
Copy link
Owner

AliSoftware commented Jul 6, 2016

Please don't forget to add an additional commit on this new branch to:

  • add an entry in the CHANGELOG.md to credit yourself
  • Update the README.md with the same changes (e.g. the README references UIKit methods like registerNib and registerClass to explain the behavior, so those will need an update

Once you've done that, with your new collaborator rights you'll be able to check those checkboxes above (I think) and close the PR at last.

[EDIT] oh wait can't reopen the PR now that the branch is deleted on your fork, of course. Never mind. Just don't forget to update the README & CHANGELOG then 😉

@AliSoftware
Copy link
Owner

Just realized Reusable still doesn't have tests nor Travis integration 😱 so that PR wasn't actually tested by Travis to ensure it compiles with Swift 3… Would be about time to implement #7 so we can be sure nothing breaks at each PR!

@phatblat
Copy link
Collaborator Author

phatblat commented Jul 6, 2016

I'll open a new PR for those metadata changes. Taking a step back, should this change be using conditional compilation markers like #if swift(>=3.0)? With those this could possible live in the master branch. Although, testing each path might be more difficult, but could probably be handled with the right Travis matrix.

@AliSoftware
Copy link
Owner

Ohh right it's definitely worth adding #ifs there I think especially given the changes are quite minimal it seems.

If you do that then not sure about the README then as the PR will indeed be merged into master and code will be valid for both Swift 2+3 so probably not worth updating the references to Swift 2 APIs in the README?
But probably worth mentioning in the README that the lib is compatible with both Swift 2+3 at least then.

@phatblat phatblat mentioned this pull request Jul 7, 2016
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants