Add .nested syntax.
#2262
Add .nested syntax.
#2262
Conversation
|
LGTM, can you also add a simple syntax test for this? https://github.com/typelevel/cats/blob/master/tests/src/test/scala/cats/tests/SyntaxSuite.scala |
|
Yes, I can add a test tomorrow. |
|
@adelbertc I added a test. |
| @@ -38,6 +38,7 @@ trait AllSyntax | |||
| with MonadErrorSyntax | |||
| with MonadSyntax | |||
| with MonoidSyntax | |||
| with NestedSyntax | |||
There was a problem hiding this comment.
unfortunately changing AllSyntax break binary compatibility on scala 2.11-. As a workaround, please let the AllSyntaxBinCompat1 above extend the new NestedSyntax
|
I changed |
| @@ -24,7 +25,7 @@ import cats.syntax.AllSyntax | |||
| * | |||
| * None of these tests should ever run, or do any runtime checks. | |||
| */ | |||
| object SyntaxSuite extends AllInstances with AllSyntax { | |||
| object SyntaxSuite extends AllInstances with AllSyntax with NestedSyntax { | |||
There was a problem hiding this comment.
Is this needed? It should be part of AllSyntax already right
There was a problem hiding this comment.
After @kailuowang's comment I removed it from AllSyntax and added it to AllSyntaxBinCompat1. So I need to add either with NestedSyntax here or with AllSyntaxBinCompat1.
There was a problem hiding this comment.
Probably AllSyntaxBinCompat1 to avoid the need to add other extensions in the future, one and done.
|
@danielkarch it could be confusing if you don't know the history. But mainly the difference between the two traits is that AllSyntax is old and already released while AllSyntaxBinCompat1 is not. |
Codecov Report
@@ Coverage Diff @@
## master #2262 +/- ##
==========================================
- Coverage 94.96% 94.67% -0.29%
==========================================
Files 333 334 +1
Lines 5799 5784 -15
Branches 218 210 -8
==========================================
- Hits 5507 5476 -31
- Misses 292 308 +16
Continue to review full report at Codecov.
|
I attended @iravid's talk at the typelevel summit and was surprised that I hadn't known about
Nestedyet.Also, I found the syntax a bit clunky. I would propose to add a
.nestedextension method so thatis equivalent to
Nested(listOfOption)Since many people
import cats.implicits._this change would also make it easier to discover.nestedthrough an IDE's syntax completion.