airbnb / okreplay

📼 Record and replay OkHttp network interaction in your tests.
Apache License 2.0
781 stars 71 forks source link

Make Optional public #79

Closed krpiotrek closed 5 years ago

krpiotrek commented 6 years ago

Hi, I'm using a custom RecorderRule which extends Recorder and implements TestRule. This is a nice, convenient way to provide some test setup/cleanup without code repetition. The problem is that Recorder method public void start(String tapeName, Optional<TapeMode> mode, Optional<MatchRule> matchRule) uses Optional - which is package private so this method cannot be called. Is there a reason behind this? If not, making Optional public would solve that issue. Or maybe switching to Kotlin's optional would be another way. Or maybe this method could accept MatchRule other way than wrapped around optional. Let me know what you think.

felipecsl commented 6 years ago

I think the way to go would be to move this class to Kotlin, as its adoption has been growing very quickly on Android, I don't see a reason why we shouldn't move it now.

krpiotrek commented 6 years ago

To make sure, you mean removing Optional class and using Kotlin nullability f.e MatchRule? or reimplementing Optional to Kotlin and making it public?

veyndan commented 5 years ago

@felipecsl Do you have a response to @krpiotrek question? I am also developing a custom recorder that extends Recorder, but I am fairly limited by Optional not being public. Personally I'd prefer to just convert the classes depending on Optional to Kotlin and using the nullability ? where appropriate. I'd be happy to work on this, but just want some clarification beforehand.

felipecsl commented 5 years ago

I mean removing Optional and relying on Kotlin nullability instead. @veyndan please feel free to send a pull request for that!