-
-
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
Use alignWith in alignMergeWith #4676
base: main
Are you sure you want to change the base?
Conversation
`alignWith` is usually overridden for efficiency
Thank you! cats/laws/src/main/scala/cats/laws/AlignLaws.scala Lines 45 to 46 in 7a30dc2
|
@satorg that law already exists. Am I missing something? |
I mean, a law that would check a correspondence between |
Done, but it seems like now unrelated tests are failing |
import cats.syntax.align.* | ||
import cats.syntax.functor.* |
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.
Just a nit but maybe these changes to the import style are unnecessary. Although it's Scalafmt duty, we use scala213source3
dialect that allows cross-version syntax constructions to be put together.
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 guess Scalafmt is not configured to update the imports
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.
Hmm, we have this
runner.dialectOverride.allowStarWildcardImport = false
and it appears to be not working (or maybe it works oppositely to my expectations)
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.
Thank you!
alignWith
is usually overridden for efficiency