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

Updating MapK to have Option alternatives #222

Merged
merged 9 commits into from
Sep 4, 2020

Conversation

tapegram
Copy link
Contributor

Update a few functions that use Option to use nullables instead.
Deprecate two functions and provide alternatives.

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.

Awesome! Thanks @tapegram.
Small nit, I think we can remove some allocations :)

arrow-core-data/src/main/kotlin/arrow/core/MapK.kt Outdated Show resolved Hide resolved
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.

Thanks @tapegram! This will safe us some allocations in the implementations 🎉

EDIT: Seems you still need to run ktlintFormat, I tried to do it for you but couldn't push to this branch.

Copy link
Member

@aballano aballano left a comment

Choose a reason for hiding this comment

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

Neat 👌

@@ -42,7 +42,6 @@ import arrow.typeclasses.Traverse
import arrow.typeclasses.Zip
import arrow.typeclasses.Unalign
import arrow.typeclasses.Unzip
import arrow.typeclasses.hashWithSalt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think this got picked up as unused by ktlintformat

Copy link
Member

@aballano aballano left a comment

Choose a reason for hiding this comment

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

Good job @tapegram, thank you!

@aballano aballano merged commit ca98830 into arrow-kt:master Sep 4, 2020
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.

3 participants