distage-example icon indicating copy to clipboard operation
distage-example copied to clipboard

Make a monomorphic and monofunctorial distage-example variants

Open Ivoyaa opened this issue 1 year ago • 8 comments

An attempt to implement https://github.com/7mind/izumi/issues/1525

Ivoyaa avatar Apr 13 '24 14:04 Ivoyaa

There are two issues with this P/R:

  1. This code is still polymorphic. It's polymorphic code abstracting from monofunctor it runs on.
  2. This P/R just replaces existing code while we want to have a separate example

I would suggest the following:

  1. Let's move our existing code into a directory like distage-example-bifunctor-tf
  2. Let's put your code into new directory, distage-example-monofunctor-tf
  3. We still need distage-example-cats-monomorphic which will use the effect (whichever it is, let's assume cats-effect) directly.

pshirshov avatar Apr 13 '24 17:04 pshirshov

There are two issues with this P/R:

  1. This code is still polymorphic. It's polymorphic code abstracting from monofunctor it runs on.
  2. This P/R just replaces existing code while we want to have a separate example

I would suggest the following:

  1. Let's move our existing code into a directory like distage-example-bifunctor-tf
  2. Let's put your code into new directory, distage-example-monofunctor-tf
  3. We still need distage-example-cats-monomorphic which will use the effect (whichever it is, let's assume cats-effect) directly.

Yep, I see. About replacing code - I initially thought to do it as Kai suggested in the original issue. To make PR here and then put it another repository, but separate directories sound good as well. To clarify: they also have to be in separate sbt modules?

Ivoyaa avatar Apr 15 '24 07:04 Ivoyaa

Yep, I think so.

pshirshov avatar Apr 15 '24 11:04 pshirshov

Sorry for enormous lines of code changes. They are caused by moving and copying src/main/resources/META-INF.native-image/docker-java.

Also, I had problems with launching GraalVM build locally, so did not actually check that commands from CI work as expected.

Ivoyaa avatar Apr 27 '24 12:04 Ivoyaa

Thank you very much! 🙏 There are a still a few small issues left before merging, though

Thanks for your time! Fixed most of the minor issues. Still not exactly sure about CI native image step. I divided it into three consequential steps but have few doubts whether it is a correct way to do it. Probably, it is better to launch them in parallel or just check bifunctor variant alone.

Ivoyaa avatar May 01 '24 10:05 Ivoyaa

Probably, it is better to launch them in parallel or just check bifunctor variant alone.

@Ivoyaa Yeah, check native image step only for bifunctor.

neko-kai avatar May 01 '24 16:05 neko-kai

@neko-kai @pshirshov hi! Fixed the CI step, can you, please, launch it again?

upd: oh, its automatic

Ivoyaa avatar May 06 '24 08:05 Ivoyaa

@pshirshov @neko-kai hi! Can you review this PR again, please? 👀

Ivoyaa avatar May 13 '24 11:05 Ivoyaa

@Ivoyaa Thank you! Sorry for the wait for merging 🙏

neko-kai avatar Jun 19 '24 17:06 neko-kai