-
Notifications
You must be signed in to change notification settings - Fork 69
Enable MIMA in build, fix binary incompatibility #195
Conversation
66c4a9c
to
0e9c0bd
Compare
Here is my solution here. add something like: object RingFunctions {
implicit class AdditionalMethods[R[_]](val r: RingFunctions[R]) extends AnyVal {
def defaultFromDouble[T](d: Double)(implicit ....): T = ...
}
} then test that the old source syntax works on Alternatively, we could add this method to What we should do is when we have a linear hierarchy use abstract class, which does not have this problem (that it is unsafe to add methods). |
Current coverage is 73.64% (diff: 100%)@@ master #195 diff @@
==========================================
Files 36 37 +1
Lines 769 774 +5
Methods 708 713 +5
Messages 0 0
Branches 61 57 -4
==========================================
+ Hits 569 570 +1
- Misses 200 204 +4
Partials 0 0
|
Okay, that fix seems to work locally and fix binary compatibility, @johnynek. |
+1 from me. @non @tixxit @denisrosset et. al. any concerns with this approach? |
Crap, binary compatibility is indeed the most important feature of algebra. I like the idea of |
@denisrosset I do prefer the companion object approach - the one thing we lose by moving it into |
We don't have to add a new class for each method, just one for each trait
that we wind up wanting to change the methods for.
I also like that Field.defaultFromDouble will work.
…On Tue, Dec 6, 2016 at 05:40 Sam Ritchie ***@***.***> wrote:
@denisrosset <https://github.com/denisrosset> I do prefer the companion
object approach - the one thing we lose by moving it into Ring is
Field.defaultFromDouble. I'm okay with that, but it may be an argument
for the DefaultMethods object.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#195 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEJdk1N6apAb25ngS-PqQwzJVVE7U9-ks5rFWXAgaJpZM4LEpe8>
.
|
@johnynek fair - and @denisrosset, we can go ahead and remove those and move the methods into the trait like you had originally done when we go to release |
we could do |
Sure, I think doing a 0.7.0 release before Spire and Algebird release makes sense. |
+1 |
Okay, great. I bumped the version, reverted the |
👍 I am away from the machine/GPG key I used to do releases. If someone else wants to do it please feel free. Otherwise I'll release Sunday when I am back. |
would someone mind kicking this? https://travis-ci.org/typelevel/algebra/jobs/181986215 This is a race condition in travis that I've seen before. |
Kicked it on the phone. If it doesn't work we'll look tomorrow |
Worked great. I
|
👍 |
Looks like we have another legit binary compatibility issue. This commit is the culprit: 24fe85a
it renames
Ring.fromDouble
todefaultFromDouble
.