Fix easy typos #3002
Fix easy typos #3002
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3002 +/- ##
==========================================
- Coverage 93.75% 93.71% -0.05%
==========================================
Files 366 366
Lines 6935 6938 +3
Branches 185 184 -1
==========================================
Hits 6502 6502
- Misses 433 436 +3
Continue to review full report at Codecov.
|
| @@ -80,7 +80,7 @@ therefore name our implicits according to the following rules: | |||
| - If the instance is for multiple type classes, use `InstancesFor` instead of a type class name. | |||
| - If the instance is for a standard library type add `Std` after the package. i.e. `catsStdShowForVector` and `catsKernelStdGroupForTuple`. | |||
|
|
|||
| As an example, an implicit instance of `Monoid` for `List` defined in the package `Kernel` should be named `catsKernelMonoidForList`. | |||
| As an example, an implicit instance of `Monoid` for `List` defined in the package `Kernel` should be named `catsKernelStdMonoidForList`. | |||
There was a problem hiding this comment.
I see you are trying to make the example consistent with the last rule. But now I think about it, I'd suggest that we remove the last rule because:
1, it's not enforced in kernel anyway due to historical reasons.
2, it's kind of unintuitive. It's not easy for contributors to just look at existing names and figure this rule out. Yes, I know the current inconsistent names are not easier to figure out a rule. But in case we will be able to fix all the names in the future, at which point this rule will stand out as unintuitive.
Not sure if we should spend much time bike shedding these rules, the names themselves provide no value whatsoever, the long names actually make the code harder to read (the type signature provides all the information). We just need them to be unique enough.
There was a problem hiding this comment.
I don't care at all about the specifics of the rules here, just that the guidelines are followed consistently, and that they are internally consistent. All of the instances in cats-kernel follow the rule you're suggesting we remove (including catsKernelStdMonoidForList itself), so if we remove the rule we're just asking for more inconsistency.
At this point I don't really care about any of this—I was just trying to take care of some low-hanging-fruit clean-up before 2.0 for an issue that bothers me and a few other people. If it's a big deal we can shelve it.
|
I'm happy either way! One thing we can all agree on is that Dotty's anonymous instances can't come soon enough. 😄 |
Another piece of #3000, posted separately here with the same motivation as #3001. These are the renamings that don't require new bincompat traits.