Skip to content
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

Restore bincompat with 2.0.0 on master #1632

Closed
mpilquist opened this issue Sep 21, 2019 · 2 comments
Closed

Restore bincompat with 2.0.0 on master #1632

mpilquist opened this issue Sep 21, 2019 · 2 comments

Comments

@mpilquist
Copy link
Member

Due to an incorrectly configured Mima setup, we didn't notice that the master branch is no longer binary compatible with the 2.0.0 release. Using #1631 as a starting point, I bisected to determine the root cause and found that it was #1610.

@diesalbla Thoughts on how to proceed? We might need to revert #1610. Apologies for missing this one.

Bisect test required changing previousVersion in build.sbt to always return "2.0.0" and then running mimaReportBinaryIssues on coreJVM project. Bisect results below:

➜  git bisect start
➜  git bisect good v2.0.0
➜  git bisect bad
Bisecting: 19 revisions left to test after this (roughly 4 steps)
[083ecf820357794f5e570496357df7133325c0ef] Use parJoin to implement `evalFilterAsync`, fix tests, add concurrency test
➜  vi build.sbt
➜  git bisect bad
Bisecting: 9 revisions left to test after this (roughly 3 steps)
[e68d18417eee01f5e469e51eeaa8b83bd3b214ae] Merge pull request #1616 from scala-steward/update/sbt-scalafix-0.9.7
➜  git bisect bad
Bisecting: 3 revisions left to test after this (roughly 2 steps)
[4fbcf4d9ba79637c44e9f90729d590222dee1ed9] Merge pull request #1617 from SystemFw/compile/resource/type
➜  git bisect bad
Bisecting: 1 revision left to test after this (roughly 1 step)
[c3cd7dfbd82ae886dc81a9dd787afde048f58c57] Merge pull request #1610 from diesalbla/unify_algebra_freec
➜  git bisect bad
Bisecting: 0 revisions left to test after this (roughly 1 step)
[4ff8cf6daf5b1c7fb581c0c614a41ef7bdf6233e] Unify FreeC and Algebra types.
➜  git bisect bad
Bisecting: 0 revisions left to test after this (roughly 0 steps)
[9789f330b927f0122eb05942bb8d00e59fff3d78] Setting version to 2.1.0-SNAPSHOT
➜  git bisect good
4ff8cf6daf5b1c7fb581c0c614a41ef7bdf6233e is the first bad commit
commit 4ff8cf6daf5b1c7fb581c0c614a41ef7bdf6233e
Author: Diego E. Alonso-Blas <[email protected]>
Date:   Sat Sep 7 15:51:01 2019 +0100

    Unify FreeC and Algebra types.

    This commit unifies the two core ADT of fs2: FreeC and Algebra.
    FreeC is a free monad with catch and interruption.
    Algebra represents a set of "abstract instructions" that may output
    values of type O, or put a value of type R on a stack.
    A Stream[F, O] consists of a `FreeC[Algebra[F, O, ?], Unit]`.

    We have noted two interesting facts of this combination:
    - In fs2, a FreeC was only used when its F[_] type was an Algebra
    - The only place in which an Algebra object would appear in a FreeC
      was through the Eval class.

    Based on these facts, and to simplify and reduce memory consumption
    (as well as some complexity) of the Stream implementation, we
    unify FreeC and Algebra, but turning "Algebra" into a subset of FreeC:

    - We add to FreeC a third parameter, where the second parameter
      represents the output of a Stream.
    - We turn the "Eval" class of FreeC into an abstract trait,
    - We remove the Algebra trat, and turn all the former Algebra
      sub-classes into sub-classes of that Eval.

    This change causes other changes:

    - The `translate` function of the FreeC.Bind is no longer needed. This
      was needed to implement `mapOutput` as a natural transformation.
      Instead, we add the `mapOutput` function to the FreeC itself.
    - We introduce another middle sub-class of FreeC, for the results.
      This is just a mere convenience, to reduce code elsewhere.
    - Since classes and types are simplified, some factory methods or
      "smart constructors", needed to lift through types, are not needed.
      Such as Algebra.step, openScope, closeScope.
    - We are now able to replace the second parameter "Nothing" in the
      inner wrapped FreeC of Pull and Stream (the output) by the O

:040000 040000 b2894623bcac86995f03d037d7e304f29af7e5d4 f238856632ebba2da13943585475202174bf543b M	benchmark
:040000 040000 168c9d21d7cfcf0ff10a2aa79783699f9213795a 5a3f907ecd75d01de58e4a33871265c190f01774 M	core
@mpilquist
Copy link
Member Author

BTW, I'm working on this revert now. Should have a PR ready in a bit.

@mpilquist
Copy link
Member Author

Update: no need to revert #1610. See comments on #1631.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant