Skip to content

Refine readme file.#28

Merged
arturdryomov merged 3 commits intomasterfrom
ad/refine-readme
Jun 15, 2018
Merged

Refine readme file.#28
arturdryomov merged 3 commits intomasterfrom
ad/refine-readme

Conversation

@arturdryomov
Copy link
Copy Markdown
Member

  • Remove text duplicating code samples.
  • Change headings structure.
  • Just general fitting things together.

Comment thread README.md
@@ -1,124 +1,106 @@
## Koptional — Minimalistic Optional type for Kotlin
# Koptional — Minimalistic `Optional` type for Kotlin
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

That looks kinda ugly tbh

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nope!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Narkomэn!

Comment thread README.md
> however in simple cases like passing `String?` through RxJava stream `Optional` is a more convenient solution.

>We also think that in many cases you can use `sealed class`es to express absent values, however in simple cases like passing `String?` through Rx stream `Optional` is a more convenient solution.
The goal of this implementation is to be convenient to use and fit Kotlin's `null`-safe type system, which resulted in:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why render null-safe as null-safe? Here it's just a term rather than null pointer value

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Just to see a sweet, sweet formatting.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

but it's ugly :D

Comment thread README.md
compile 'com.gojuno.koptional:koptional-rxjava2-extensions:put-some-version'
implementation "com.gojuno.koptional:koptional-rxjava2-extensions:$koptional_version"
```

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you add Reactor pls?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Sure. Nuked a separate README.md and inlined it here as well.

Comment thread README.md Outdated
val f = optional.toNullable() ?: "fallback"
```
#### Check if `Optional` is `Some` or `None`
#### Use [Smart Cast](http://kotlinlang.org/docs/reference/typecasts.html#smart-casts)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's not use word Use? It reads like we're forcing users to use these features :)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The idea was to use verbs for the Usage section — create, convert, leverage and so on. I’m not opposed to change it though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah I understand, I think it's fine in previous headers, but kinda odd here

"Interop with Java" doesn't have verb and looks fine :)

Comment thread README.md Outdated
Koptional supports [destructuring](https://kotlinlang.org/docs/reference/multi-declarations.html).

Destructuring has same effect as calling `toNullable()`.
#### Use [Destructuring](https://kotlinlang.org/docs/reference/multi-declarations.html)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Especially here, destructuring is not required in many cases but this doc sorta says USE IT

Comment thread README.md Outdated
Use the static `Optional.toOptional()` to wrap an instance of `T` into `Optional<T>`.

#### Filter only `None` values emitted by RxJava 2 or Project Reactor
### Work with RxJava 2
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe just "RxJava 2 Extensions"?

@arturdryomov
Copy link
Copy Markdown
Member Author

@artem-zinnatullin, addressed most of your comments.

Copy link
Copy Markdown
Contributor

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@arturdryomov arturdryomov merged commit abdeb01 into master Jun 15, 2018
@arturdryomov arturdryomov deleted the ad/refine-readme branch June 15, 2018 06:05
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