optics-dev / Monocle

Optics library for Scala
https://www.optics.dev/Monocle/
MIT License
1.65k stars 205 forks source link

Support both cats and scalaz #419

Closed julien-truffaut closed 7 years ago

julien-truffaut commented 7 years ago

This is a replacement for #235

I am not excited at the idea of supporting both scalaz and cats but I think it is the right thing to do for our users. A lot of people have asked for a monocle version depending on cats but at the same time we have a large user base using scalaz. Maybe we will be able to drop scalaz support when scalaz 8 is out as it should have a full implementation of optics.

As far as I know, they are 3 approaches to support both scalaz and cats:

  1. shims a compatibility layer between scalaz and cats
  2. a prepocessor like what was done in doobie
  3. master using scalaz and a separate branch using cats

1) I don't think it is applicable for monocle because we want to support optics for data type defined in cats and scalaz. 2) is quite attractive as we can use a single code base but it means IDE support will be completely useless. So it is likely to limit people contribution. 3) it is very painful as it means we need to backport any change from master to cats branch and I don't know how can we support documentation for both. However, we will have complete control over both branches e.g. we could use Either in cats branch but keep \/ in scalaz branch.

At the moment, I believe 3 is the best solution for monocle but I would love if someone can propose a better option.

deontologic commented 7 years ago

Not sure of this issue's status, but dealing with the cats-scalaz schism might be easier for Monocle than libraries like Doobie or http4s.

For now, support for cats could be done in a separate branch, and when cats hits v1, be used as default. Scalaz support (in whatever form) could continue until v8 is released, and then be deprecated.

julien-truffaut commented 7 years ago

@deontologic Yeah I agree. It is quite a long task though and no one has picked it up yet.

mdedetrich commented 7 years ago

It sounds like the best long term option is to only support cats since Scalaz 8 is coming with its own optics library. Until then I agree with supporting the latest scalaz 7 stable + cats (with continuously updating cats as it continues to release stable versions after 1.x)

Only thing is that I suspect that the move from Scalaz 7 to Scalaz 8 would be quite painful for users as well, so there might be some interim period where Scalaz users are stuck with outdated Monocle versions because they are stuck on Scalaz 7, but I don't think that this should be such a huge issue

sellout commented 7 years ago

I am struggling with this same question in Matryoshka.

Re: “we want to support optics for data type defined in cats and scalaz,” I think shims still works for that case, because you end up with monocle-cats and monocle-scalaz projects that contain those optics. I think the bigger problem with using shims is dealing with places where you use the FP-lib data types in the core of the library. I’m not sure how tangled that is for Monocle, but it’s been a pain for Matryoshka.

julien-truffaut commented 7 years ago

@sellout is almost done on his heroic task to port Monocle to cats. Here are a few questions we need to answer:

Personally, I can see two options:

  1. we move the current code in master into a scalaz branch and support cats in master. We add a scalaz suffix to all scalaz artifacts (e.g. monocle-core uses cats but monocle-core-scalaz uses scalaz).
  2. we let master as it is (i.e. scalaz) and support cats in a cats branch. We add a cats suffix to all cats artifacts (e.g. monocle-core-cats uses cats but monocle-core uses scalaz).

I think we should try to support both scala and cats for a full minor version (e.g. 1.5.x). Then we can decide if we have enough will to continue the experimentation or we let someone else maintain scalaz part.

milessabin commented 7 years ago

If scalaz 8 is going to have its own optics then I think your option (1) is the way to go.

tpolecat commented 7 years ago

I agree that (1) above is the way to go. It's great that @sellout has done so much work on this.

Somewhat related, I will be doing away with shims/yax later this year and will standardize on cats for future development of the projects I work on, so for me personally Monocle for cats will be a great thing to have.

drdozer commented 7 years ago

Monocle for cats, especially if it could use scala-meta, would be great - the lack of monocle for cats is the reason that I have one project with both scalaz and cats in.

mdedetrich commented 7 years ago

Option 1 as well

fsvehla commented 7 years ago

Option 1

weihsiu commented 7 years ago

+1 on option 1

On Tue, Apr 4, 2017, 9:04 PM Ferdinand Svehla notifications@github.com wrote:

Option 1

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/julien-truffaut/Monocle/issues/419#issuecomment-291493037, or mute the thread https://github.com/notifications/unsubscribe-auth/AAB5GwV-yAuk0zgiR8Nmo9oHkNyXxmpgks5rsj_qgaJpZM4KwulT .

amrhassan commented 7 years ago

Option 1

S11001001 commented 7 years ago

1 is fine, provided a major version bump, not a minor one. If you would prefer a minor version bump, 2 would be less confusing for existing users (who won't necessarily look askance at automatically bumping a minor version).

alonsodomin commented 7 years ago

Option 1

julien-truffaut commented 7 years ago

It seems option 1 is the most popular with @S11001001 suggestion to bump to 2.0.0.

I received a few messages of people worried about Monocle dropping scalaz support. I personally don't want it and I have nothing against scalaz, I believe it is an awesome library. However, I doubt we will have the motivation to maintain 2 versions of Monocle forever. So I propose we do it at least for 2.0.x and see how it goes. If it is too painful, then we will look for a maintainer for scalaz branch. If no one is motivated, we will drop the support for scalaz in the next minor version (we can of course provide patches to earlier versions if required).

Does it sound reasonable? Feel free to comment here or send me a PM

mdedetrich commented 7 years ago

@julien-truffaut Perfectly happy with this approach, as mentioned before Scalaz 8 will have its own Optics support, and there is nothing stopping people doing minor bumps for the Monocle 1.x version which has Scalaz 7 support

boggle commented 7 years ago

What's the status of this?

julien-truffaut commented 7 years ago

@boggle it is almost ready, I think @sellout is waiting for a PR to be merged and release in cats

zsolt-donca commented 7 years ago

What is the status of this? If there is still a related PR in cats, can you provide a link? Thanks!

julien-truffaut commented 7 years ago

@zsolt-donca @sellout told me that it is likely already merged in cats 1.0.0-MF. So we need to update monocle cats branch accordingly and wait for 1.0.0 release.

julien-truffaut commented 7 years ago

1.5.0-cats-M1 is released which depends on cats 1.0.0-MF