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

[WIP] Swift 2.2 #243

Closed
wants to merge 3 commits into from
Closed

[WIP] Swift 2.2 #243

wants to merge 3 commits into from

Conversation

NachoSoto
Copy link
Contributor

I've changed all types for __FILE__ to FileString, and it made that always StaticString to match XCTest.

This was originally brought up in #242: XCTest methods now take StaticString (see swiftlang/swift#888 for details). Unfortunately this won't compile now because StaticString is not available in Objective-C!

Originally brought up in Quick#242, `XCTest` methods now take `StaticString` (see swiftlang/swift#888 for details). Unfortunately this won't compile now because `StaticString` is not available in Objective-C!
@briancroom
Copy link
Member

Hey @NachoSoto, thanks for taking a stab at this. I'm disappointed that nobody (including myself!) realized the ObjC bridging implications while discussing the StaticString change in the XCTest overlay previously. I've left a comment on the PR asking for additional input on this issue.

Assuming that the change doesn't get reverted, though, does anybody have thoughts on how to work around this change? The only thing I've got so far would be to switch to bypass the Swift overlay and use the Objective-C XCTestCase.recordFailureWithDescription method for the failure reporting. This would require us to be able to reference the currently-running XCTestCase instance, which I believe we could track using XCTestObservationCenter.

@modocache
Copy link
Member

I'm disappointed that nobody (including myself!) realized the ObjC bridging implications while discussing the StaticString change in the XCTest overlay previously.

I feel this would've taken a great deal of foresight, from someone with a ton of context on StaticString, the SDK overlay, and how the file parameter is used by third-party testing tools. There's hardly anyone with that much breadth of knowledge--although you're fast becoming that person! Don't let this one setback get you down, your work is much appreciated!! And thanks for commenting on swiftlang/swift#888. 💯

@briancroom
Copy link
Member

Hah, disappointment was probably too strong a word. Let's rather call it mild annoyance at being caught off guard by the ramifications of a change that I was involved in 😂

Anyway, @NachoSoto and (@ratkins), I'm curious to hear how urgent you feel it is for us to try to get support for Xcode 7.3 beta 2? Even if swiftlang/swift#888 ends up getting reverted (I'm not holding my breath! And I'm not even fully convinced that it should be, at this point), it will be a few weeks until the next beta rolls around.

@NachoSoto
Copy link
Contributor Author

Thanks guys! There's definitely no rush for this beta. I just usually like to get a head start to find things like this (or like many other bad things I've found over the years 😅).

I think we should consider this important, otherwise I can already see a 7.3 GM with this bug that would mean Nimble can't support Objective-C anymore, or that compromises have to be made :P

@ratkins
Copy link

ratkins commented Jan 28, 2016

My situation is, I want to try running our suite under 7.3b2 so I can tell if it fixes other (Xcode) bugs that are causing our CI to fail—so I can feed that back in to the 7.3 beta. But at the same time I need to keep our master branch 7.2 compatible so we can release through TestFlight. I'm not sure what the best approach to this would be for us (with the added constraint that I'm only going to be on the project for another week, before sending the client home with whatever path forward I can come up with.)

@briancroom
Copy link
Member

I'm traveling right now and don't have connectivity for my laptop, but I've put s branch together with one approach to resolving this which I hope to push for review tomorrow. (Just a heads up in case anyone was was considering taking a stab at it!)

@NachoSoto
Copy link
Contributor Author

Continued in #244.

@NachoSoto NachoSoto closed this Jan 31, 2016
@NachoSoto NachoSoto deleted the swift-2.2 branch February 1, 2016 22:16
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.

None yet

4 participants