Skip to content

Conversation

@jrose-apple
Copy link
Contributor

In the SDK shipped with Swift 3, NSXMLNodeKind's enumerator NSXMLDTDKind was mistakenly annotated with NS_SWIFT_NAME(DTDKind) instead of NS_SWIFT_NAME(dtd). We'd like to fix that in a later version of the SDK, but we need to maintain Swift 3 compatibility on the offchance someone is using the bad name.

There's already an unversioned SwiftName API note for NSXMLDTDKind to mark it as 'dtd', so with this change we can just not mention it in the headers at all.

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test

@phausler
Copy link
Contributor

so this will be a breaking change without an update to the Foundation headers right?

@jrose-apple
Copy link
Contributor Author

It's not for two different reasons:

After this change, Foundation can either remove its NS_SWIFT_NAMEs from NSXMLDTDKind entirely, or fix it in the header and then remove the unversioned entry from the API notes. We're stuck with this new versioned one for compatibility reasons, though.

@phausler
Copy link
Contributor

TBH I would much rather remove that ugliness from our headers (this particular case is hard to read). If we did that (removed the NS_SWIFT_NAME) it would need a synchronized submission then?

Just making sure that it isn't changing the name in swift 3 or 4 'cause that would require a full API review.

@jrose-apple
Copy link
Contributor Author

I don't think we need a fully synchronized submission, but Foundation will have to wait until this change is submitted first. Still, this is planning to change the name in Swift 4; it's just changing it to "dtd" which it was always intended to be in Swift 3. (And presumably was reviewed as such.)

@jrose-apple
Copy link
Contributor Author

@swift-ci Please smoke test Linux

@phausler
Copy link
Contributor

@parkera should we hold off on this until proposals go through for the swift 4 naming to dtd?

@jrose-apple
Copy link
Contributor Author

If you'd prefer I can change it to "DTDKind" everywhere for now, just to get the Clang patch unblocked.

@phausler
Copy link
Contributor

I think that would be the best route so that we can get it approved as an API change - it seems reasonable but there may be other issues yet unforeseen (iirc there is ambiguity with dtd)

In the SDK shipped with Swift 3, NSXMLNodeKind's enumerator
NSXMLDTDKind was mistakenly annotated with `NS_SWIFT_NAME(DTDKind)`
instead of `NS_SWIFT_NAME(dtd)`. We'd like to fix that in a later
version of the SDK, but we need to maintain Swift 3 compatibility on
the off-chance someone is using the bad name. If we /do/ decide to
change this, we can undo this change (going back to an API note
SwiftName of 'dtd') and add a Swift-3-versioned SwiftName of 'DTDKind'
instead.
@jrose-apple
Copy link
Contributor Author

Okay, does this look good?

@swift-ci Please smoke test

@jrose-apple
Copy link
Contributor Author

(ping)

@phausler
Copy link
Contributor

Yea go for it, I will see if we can follow up on this in Foundation separately.

@jrose-apple
Copy link
Contributor Author

Thank you!

@jrose-apple jrose-apple merged commit a3eb1a7 into swiftlang:master Jan 24, 2017
@jrose-apple jrose-apple deleted the NSXMLDTDKind branch January 24, 2017 23:11
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