cats icon indicating copy to clipboard operation
cats copied to clipboard

Published 20 hours ago • verified publisher icon typelevel

Optimize `Alternative` (part 4): add `prependK`/`appendK` specializations for Cats monad transformers

Open satorg opened this issue 2 years ago • 8 comments

Adds optimized specializations for the new prependK/appendK methods introduced in the initial PR (#4014).

Previous PRs:

  • #4052.
  • #4055

satorg avatar Sep 16 '22 05:09 satorg

The PR is created as "Draft" because this work was done quite a long ago and I just want to make sure I didn't lose something after lots of merges and rebases. But perhaps it is good to go already.

satorg avatar Sep 16 '22 05:09 satorg

I've made a deep inhale and re-reviewed the changes here. I would say they look good to me and seems all the tests and Mima checks are happy as well. So I would dare to claim the PR is ready to go)

satorg avatar Oct 09 '22 08:10 satorg

I think that all the concerns have been addressed here. Could you take one more look please?

satorg avatar Oct 20 '22 17:10 satorg

Sorry, haven't forgotten. Rather than "one more look" it needs "one proper look" actually 😅

armanbilge avatar Oct 27 '22 02:10 armanbilge

@armanbilge I added direct NonEmptyAlternativeLaws tests for IRWST type. But besides that I could not refrain from cleaning that file up a little bit:

  1. class ReaderWriterStateTSuite renamed to IndexedReaderWriterStateTSuite to make it corresponding to the file name.
  2. "Organize imports" from VSCode.
  3. The test for AlternativeLaws is made looking more alike other law tests around it.
  4. Fixed compiler warnings (just in that file – yes, I did it there too!)

satorg avatar Oct 31 '22 08:10 satorg

Nowadays, there's a lot of hype on AI technologies around – lots of people try to ruminate on what the AI can or cannot do and how it can be useful or even dangerous to all of us.

I personally think though, that at least one of the things that the AI could really help with – is to review such huge and boring PRs like this one.

satorg avatar Jun 14 '23 17:06 satorg

is to review such huge and boring PRs like this one.

Sorry I dropped the ball on reviewing this 😢 also I think your force-pushes erased my review progress 😅

armanbilge avatar Jun 26 '23 18:06 armanbilge

Hi, @armanbilge, @johnynek , Sorry for the ping. Just to confirm with you – does it make sense to maintain this PR? Actually, I'm not asking about rushing reviewing it, but if you think it still could be useful (somewhen later), then I could allocate some time to actualize it – rebase on main, resolve conflicts, etc. Otherwise, I am fine just to go close it, no problem from my side – it does not look that critical anyway. Thanks!

satorg avatar Mar 01 '24 03:03 satorg