-
Notifications
You must be signed in to change notification settings - Fork 72
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
Improve documentation and tests #348
Conversation
@@ -1,10 +1,10 @@ | |||
import UIKit | |||
|
|||
/// A proxy cell that is used for composite views inside other Spotable objects | |||
class CarouselComposite: UICollectionViewCell, Composable { | |||
public class CarouselComposite: UICollectionViewCell, Composable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zenangst I see a lot of public
in this PR 😄 Why is that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The damn linter is complaint about comments on internal implementations.
Also some of the things probably should be public to being with ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would limit the amount of open
and public
to simplify the public API, so in the end there are only functionality that should be accessible. But if it's supposed to be public
, then we're good here.
I see tests are failing 😄 |
@vadymmarkov only on the damn PR... TRAVIS!!! 🔨 |
And now the tests pass 🚦 |
Current coverage is 69.99% (diff: 84.84%)
|
This PR aims to slightly improve tests and the source code documentation.
It also resolves a Segfault that seems to have surfaced with Swift 3.0.1
This seems to be resolved by guarding for
keyPath
and reversing the compare statement.