Skip to content

Conversation

@copumpkin
Copy link
Contributor

@copumpkin copumpkin commented Sep 13, 2018

I think this might have been an artifact of the old CFLite redactions? Either way, these headers are now available in this repository. The real CoreFoundation guards these includes with #ifndef CF_OPEN_SOURCE but that doesn't seem relevant here. There are also other includes guarded
by that which are not (yet? 😆) available in this repository, so I did not include them here.

cc @millenomi @parkera it's hard for me as an outsider to know the "right" thing to do around CF_OPEN_SOURCE avoidance. I see a little bit of it still in this repo, but it's hard for me to know if that's appropriate here (in order to be more similar to the official CF) or if it's just going to be unconditionally available from now on. Any advice if I encounter more stuff like this?

Edit: I'm also unclear on how I should be keeping this CoreFoundaiton.h in sync with the one in SwiftRuntime. Should I make the same change there?

@copumpkin
Copy link
Contributor Author

I'm honestly not 100% sure this is necessary, because other headers (like CFTimeZone.h for example) do #include these headers indirectly, but it seemed good to not rely on the internals of those and to match the official framework more closely wherever possible.

@parkera
Copy link
Contributor

parkera commented Sep 13, 2018

I think we need to make the same change in both CF.h headers.

I agree with you that the exclusion is an artifact of before we had these files available in the open source CF.

@copumpkin
Copy link
Contributor Author

Cool, I'll update the other header, thanks!

I think this might have been an artifact of the old CFLite redactions?
Either way, these headers are now available in this repository. The real
CoreFoundation guards these includes with `#ifndef CF_OPEN_SOURCE` but
that doesn't seem relevant here. There are also other includes guarded
by that which are not (yet? 😆) available in this repository, so I did
not include them here.
@copumpkin copumpkin force-pushed the add-back-unnecessary-redactions branch from 452bd6a to 1e17e6d Compare September 13, 2018 19:17
@copumpkin
Copy link
Contributor Author

@parkera turns out the other header already had CFAttributedString.h so I just added the notification center

@parkera
Copy link
Contributor

parkera commented Sep 13, 2018

@swift-ci test and merge

@copumpkin
Copy link
Contributor Author

Hmm, I think @swift-ci might be stuck

@spevans
Copy link
Contributor

spevans commented Sep 14, 2018

@swift-ci test and merge

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.

4 participants