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

Provide Nullable alternatives to APIs that require Option #239

Closed
wants to merge 16 commits into from
Closed

Provide Nullable alternatives to APIs that require Option #239

wants to merge 16 commits into from

Conversation

LordRaydenMK
Copy link
Contributor

@LordRaydenMK LordRaydenMK commented Sep 20, 2020

In order to move Option to a separate module first we need to remove all arrow-core-data and arrow-core usages from internal and public APIs and provide equivalent APIs with nullable types.

Replaces internal Option usages with nullable types.
Adds Nullable counterparts to Option APIs in typeclasses
Remove Option examples from API docs
Remove some deprecated methods with Option

Note: I might have missed something, if that is the case I will do another PR.

@nomisRev
Copy link
Member

Looks great so far to me 🙌

@LordRaydenMK LordRaydenMK marked this pull request as ready for review September 26, 2020 10:01
@LordRaydenMK LordRaydenMK changed the title [WIP] Replace Option with Nullable Provide Nullable alternatives to APIs that require Option Sep 26, 2020
@LordRaydenMK
Copy link
Contributor Author

Looks like the meta annotation processor doesn't play well with nullable types:

//Foldable.kt
  /**
   * Get the first element of the foldable, or null
   */
  fun <A> Kind<F, A>.firstOrNull(): A? =
    findNullable { true }
//ListKFoldable.kt (in the build folder)

fun <A> Kind<ForListK, A>.firstOrNull(): A = arrow.core.ListK.foldable().run {
  [email protected]<A>() as A
}

the return type should be A? instead of A. Should I create a ticket for this? I would fix it but I have no idea where to even begin to look for it 😄

@rachelcarmena
Copy link
Member

Thanks @LordRaydenMK !! Yes, please, create the issue and I'll take a look 👍

@LordRaydenMK
Copy link
Contributor Author

Looks like it's already there: #147

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.

Much needed, thanks @LordRaydenMK !

@pablisco pablisco self-requested a review September 26, 2020 18:45
Copy link
Contributor

@pablisco pablisco left a comment

Choose a reason for hiding this comment

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

Just a couple of thoughts :) Great work 💪

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.

Great work @LordRaydenMK! Thanks 👏 👏 🎉

@rachelcarmena
Copy link
Member

Looks like it's already there: #147

Oh, I see, thanks @LordRaydenMK I thought it could be related with other dependencies that I updated recently. So it would be necessary to upgrade KotlinPoet to make it work.

@nomisRev
Copy link
Member

e: /Users/runner/work/arrow-core/arrow-core/arrow-core/src/main/kotlin/arrow/core/extensions/validated/foldable/ValidatedFoldable.kt: (337, 33): 2 type arguments expected for fun <E, A> Kind<Kind<ForValidated, E>, A>.firstOption(): Option

Oops, not sure what I did wrong in the merge 😅 I only fixed a doc with a conflict.

@LordRaydenMK
Copy link
Contributor Author

e: /Users/runner/work/arrow-core/arrow-core/arrow-core/src/main/kotlin/arrow/core/extensions/validated/foldable/ValidatedFoldable.kt: (337, 33): 2 type arguments expected for fun <E, A> Kind<Kind<ForValidated, E>, A>.firstOption(): Option

Oops, not sure what I did wrong in the merge sweat_smile I only fixed a doc with a conflict.

I am happy to fix once #147 is resolved (as it is blocking merging this).

@nomisRev
Copy link
Member

@LordRaydenMK I'm not sure if you followed but we're in the process of removing @extensions which will resolve it as well.
Then we can continue and merge this PR. Sorry for the long wait.

Ref: #267

@rachelcarmena
Copy link
Member

Hi fellow contributor 👋

Arrow repositories has been reunified again into arrow so this repository is going to be archived.

Please, if you want to replay this pull request on arrow repository, follow these steps:

# Adapt your branch to be able to be merged in the new arrow repository
cd arrow-core
git-filter-repo --to-subdirectory-filter arrow-libs/core
cd ..

# Create a new branch in arrow repository and merge your content
git clone [email protected]:arrow-kt/arrow.git
cd arrow
git checkout -b <new-branch>
git remote add arrow-core ../arrow-core
git fetch arrow-core
git merge --no-edit --allow-unrelated-histories arrow-core/<your-branch>
# check and create the pull request again on arrow repository

Please, let us know if we can help you!

Thank you so much for your contributions!! 🙌

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.

5 participants