Skip to content
This repository has been archived by the owner on Feb 24, 2021. It is now read-only.

Deprecate @higherkind & @extension for Sequence/SequenceK #329

Merged
merged 41 commits into from Feb 9, 2021

Conversation

franciscodr
Copy link
Contributor

  • Align
  • Alternative
  • Applicative
  • Apply
  • Crosswalk
  • Eq (EqK)
  • Foldable
  • Functor
  • FunctorFilter
  • Monad
  • MonadCombine
  • MonadFilter
  • MonadLogic
  • MonadPlus
  • Monoid (MonoidK)
  • Monoidal
  • Repeat
  • Semialign
  • Semigroup (SemigroupK)
  • Semigroupal
  • Traverse
  • Unalign
  • Unzip
  • Zip

@franciscodr franciscodr requested a review from a team February 5, 2021 15:30
@@ -148,8 +138,7 @@ fun <A> Sequence<A>.optional(): Sequence<Option<A>> =
@Deprecated(
"@extension kinded projected functions are deprecated",
ReplaceWith(
"guard(arg0)",
"arrow.core.extensions.sequence.alternative.Sequence.guard"
"if (arg0) sequenceOf(Unit) else emptySequence()"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be promoting in these replacements apis we are providing such as Sequence.unit otherwise we should question if Sequence.unit has value over sequenceOf(Unit)?

Copy link
Member

@nomisRev nomisRev Feb 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do Sequence.unit since we don't own Sequence, right?

I frequently use Either.unit() if I need to use Either in some kind of loop, and it's become a habit to use Either.unit() whenever I need Right(Unit). In general, however, I'd say that it's probably not very useful since it's a micro-optimization.

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. A couple of small things that need to be fixed added as suggestions.
Must've felt like a never-ending task 🙈 Thank you @franciscodr!

Copy link
Member

@nomisRev nomisRev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All comments were addressed. Thanks @franciscodr 👍 👏 👏

Copy link
Member

@raulraja raulraja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @franciscodr !

@franciscodr franciscodr merged commit e90b855 into master Feb 9, 2021
@franciscodr franciscodr deleted the paco-deprecate-extensions-sequencek branch February 9, 2021 13:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants