Skip to content

Remove LDCodecWithInfallibleEncode - #248

Open
morgen-peschke wants to merge 7 commits into
typelevel:mainfrom
morgen-peschke:clean-up-ldcodec
Open

morgen-peschke wants to merge 7 commits into
typelevel:mainfrom
morgen-peschke:clean-up-ldcodec

Conversation

@morgen-peschke

Copy link
Copy Markdown
Contributor

We're papering over something that probably shouldn't be papered over, and it makes the whole thing messy.

Much better to be open about LDValue's inability to encode certain values, and keep the type hierarchy clean.

We're papering over something that probably shouldn't be papered over, and it makes the whole thing messy.
Much better to be open about LDValue's inability to encode certain values, and keep the type hierarchy clean.

@rossabaker rossabaker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

or die is giving me Perl flashbacks.

Comment thread core/src/main/scala/org/typelevel/catapult/FeatureKey.scala
Really not sure why this wasn't deleted before, it wasn't showing in my editor, so I'm guessing some sort of odd IntelliJ bug?
- Fix a Scala 3 thing that was making the tests mad
- Made LDCursor a bit easier to work with
@morgen-peschke

Copy link
Copy Markdown
Contributor Author

or die is giving me Perl flashbacks.

I've always been fond of that wording 😁

@zarthross zarthross left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the naming can be fixed on the 'orDie', usually we use 'unsafe' as the vocabulary for those types of methods right?

Comment thread core/src/main/scala/org/typelevel/catapult/FeatureKey.scala Outdated
Comment thread core/src/main/scala/org/typelevel/catapult/FeatureKey.scala Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants