Updates the repository for Swift 3 compatibility #24

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet
2 participants
@AngeloStavrow
Contributor

AngeloStavrow commented Aug 11, 2016

Merging this into the correct branch per original PR. 馃憤

@sxg

This comment has been minimized.

Show comment
Hide comment
@sxg

sxg Aug 12, 2016

Owner

Thanks so much! I'll be reviewing this over the weekend before I merge into the swift3 branch.

Owner

sxg commented Aug 12, 2016

Thanks so much! I'll be reviewing this over the weekend before I merge into the swift3 branch.

@AngeloStavrow

This comment has been minimized.

Show comment
Hide comment
@AngeloStavrow

AngeloStavrow Aug 12, 2016

Contributor

Happy to help! I look forward to your comments, I'm a bit new to this. 馃槃

Contributor

AngeloStavrow commented Aug 12, 2016

Happy to help! I look forward to your comments, I'm a bit new to this. 馃槃

@sxg

This comment has been minimized.

Show comment
Hide comment
@sxg

sxg Aug 13, 2016

Owner

I updated the .travis.yml file to run the unit tests on TravisCI, and it looks like there are some issues with Nimble. Here's a link to the unit test issues. For some reason TravisCI is passing the whole build, but a lot of the individual unit tests are failing. It looks like a lot of little errors - do you mind taking a look at them?

Owner

sxg commented Aug 13, 2016

I updated the .travis.yml file to run the unit tests on TravisCI, and it looks like there are some issues with Nimble. Here's a link to the unit test issues. For some reason TravisCI is passing the whole build, but a lot of the individual unit tests are failing. It looks like a lot of little errors - do you mind taking a look at them?

@AngeloStavrow

This comment has been minimized.

Show comment
Hide comment
@AngeloStavrow

AngeloStavrow Aug 14, 2016

Contributor

Sure! I'll have a look at some point this week鈥擨'm not familiar with Nimble, so it may take me a little time to fix.

Contributor

AngeloStavrow commented Aug 14, 2016

Sure! I'll have a look at some point this week鈥擨'm not familiar with Nimble, so it may take me a little time to fix.

@sxg

This comment has been minimized.

Show comment
Hide comment
@sxg

sxg Aug 14, 2016

Owner

No worries. I'm taking a look at this too.

Owner

sxg commented Aug 14, 2016

No worries. I'm taking a look at this too.

@sxg

This comment has been minimized.

Show comment
Hide comment
@sxg

sxg Aug 14, 2016

Owner

I tried making some fixes to Nimble for Swift 3 compatibility, but it looks like there are some ongoing issues that may or may not be easily fixable. I'm thinking it might be a good idea to migrate away from Nimble and just use XCTest. What do you think of that? There aren't too many unit tests, so it won't be that much work, and I'd be able to remove another dependency from the project so we can avoid future compatibility issues. If you have better luck than me with updating Nimble, then definitely let me know and we can stick to the original plan.

Owner

sxg commented Aug 14, 2016

I tried making some fixes to Nimble for Swift 3 compatibility, but it looks like there are some ongoing issues that may or may not be easily fixable. I'm thinking it might be a good idea to migrate away from Nimble and just use XCTest. What do you think of that? There aren't too many unit tests, so it won't be that much work, and I'd be able to remove another dependency from the project so we can avoid future compatibility issues. If you have better luck than me with updating Nimble, then definitely let me know and we can stick to the original plan.

@sxg

This comment has been minimized.

Show comment
Hide comment
@sxg

sxg Aug 15, 2016

Owner

I actually went ahead and did the migration to XCTest from Nimble. It didn't look like Nimble would be an easy migration to Swift 3, and ForecastIO isn't really benefiting too much from Nimble anyway. It just makes the unit test syntax a bit more readable. I've incorporated all of our combined changes into the swift3 branch. I'm going to close this pull request, but let me know what you think of this!

Owner

sxg commented Aug 15, 2016

I actually went ahead and did the migration to XCTest from Nimble. It didn't look like Nimble would be an easy migration to Swift 3, and ForecastIO isn't really benefiting too much from Nimble anyway. It just makes the unit test syntax a bit more readable. I've incorporated all of our combined changes into the swift3 branch. I'm going to close this pull request, but let me know what you think of this!

@sxg sxg closed this Aug 15, 2016

@AngeloStavrow

This comment has been minimized.

Show comment
Hide comment
@AngeloStavrow

AngeloStavrow Aug 15, 2016

Contributor

I think that makes sense鈥攚hile testing is obviously important, I can't help but feel that there's also something to be said for having less dependencies to worry about (especially when you're working in a language that's evolving as quickly as Swift).

Contributor

AngeloStavrow commented Aug 15, 2016

I think that makes sense鈥攚hile testing is obviously important, I can't help but feel that there's also something to be said for having less dependencies to worry about (especially when you're working in a language that's evolving as quickly as Swift).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment