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

rxcocoa build fix on xcode 9 #1341

Merged
merged 3 commits into from
Jul 18, 2017
Merged

rxcocoa build fix on xcode 9 #1341

merged 3 commits into from
Jul 18, 2017

Conversation

anonymcek
Copy link

I am not sure about other classes and their visibility but the RxPickerViewSequenceDataSource being private doesn't work on XCode 9. Actually I don't understand how it could build at all without being public.

@@ -31,12 +31,12 @@ private class RxPickerViewArrayDataSource<T>: NSObject, UIPickerViewDataSource,
}
}

private class RxPickerViewSequenceDataSource<S: Sequence>
public class RxPickerViewSequenceDataSource<S: Sequence>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @anonymcek ,

I don't think any of these elements should be public.

Can you please try just removing private identifier from RxPickerViewSequenceDataSource and from RxPickerViewArrayDataSource?

Copy link
Author

@anonymcek anonymcek Jul 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RxPickerViewArrayDataSource can stay private, should I remove it anyway?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that anyone understands anymore what is by design and what works by pure accident here so I think it would be safer to also remove private from RxPickerViewArrayDataSource.

@sergdort
Copy link
Contributor

I wonder if it's because of new Swift access control changes of private and fileprivate

I feel like it should work with fileprivate

@anonymcek
Copy link
Author

anonymcek commented Jul 18, 2017

it is referenced from RxUIPickerView+Rx so it wouldn't be visible with fileprivate either

I can add internal just to set the scope for entire target explicitly...

@anonymcek anonymcek closed this Jul 18, 2017
@anonymcek anonymcek reopened this Jul 18, 2017
@anonymcek
Copy link
Author

does anyone have a clue what is wrong with that tests?

@anymuse anymuse mentioned this pull request Jul 18, 2017
17 tasks
@kzaher kzaher merged commit 3722e09 into ReactiveX:develop Jul 18, 2017
@sergdort
Copy link
Contributor

Hm this is so weird.. Because we do not reference RxPickerViewSequenceDataSource from UIPickerView+Rx ¯_(ツ)_/¯

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.

3 participants