-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add sequenceFilter
syntax
#2600
Conversation
@@ -77,3 +78,5 @@ trait AllSyntaxBinCompat2 | |||
with ValidatedSyntaxBincompat0 | |||
|
|||
trait AllSyntaxBinCompat3 extends UnorderedFoldableSyntax | |||
|
|||
trait AllSyntaxBinCompat4 extends TraverseFilterSyntaxBinCompat0 |
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.
AllSyntaxBinCompat3
was introduced since last release, you should be able to just use it.
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.
Ah, wonderful. I was actually thinking about that but hadn't checked before making this PR. I'll move the extension there, then.
I also plan on providing some test(s).
@kailuowang is there a chance to get this in 1.5.0? I'll merge the bincompat syntax traits now. |
Looks like I'm missing the new instances ( |
@@ -2,3 +2,12 @@ package cats | |||
package syntax | |||
|
|||
trait TraverseFilterSyntax extends TraverseFilter.ToTraverseFilterOps | |||
|
|||
trait TraverseFilterSyntaxBinCompat0 { |
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.
We also want this to go to cats.syntax.all
and cats.implicits
right?
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.
also can we add at least a doctest for it?
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.
It was in cats.syntax.all
:) I added it to cats.implicits now, also made a doctest. Because there's been a release in the meantime, I added back the new bincompat trait. I'll push the changes once sbt gives me a green light.
ideally we want to test all non-bug fix changes in RC before rolling out. So if it's not urgent let's wait for 1.6. I plan to shorten the release cadence to 1-2 months. |
Codecov Report
@@ Coverage Diff @@
## master #2600 +/- ##
==========================================
+ Coverage 95.12% 95.12% +<.01%
==========================================
Files 363 364 +1
Lines 6704 6706 +2
Branches 289 289
==========================================
+ Hits 6377 6379 +2
Misses 327 327
Continue to review full report at Codecov.
|
Sure, that fits me. I'll update this PR soon :) |
@kailuowang please take a look :) |
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.
LGTM, thanks so much for keeping this one updated.
No description provided.