Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

parametrize types on Option #707

Closed
wants to merge 2 commits into from
Closed

parametrize types on Option #707

wants to merge 2 commits into from

Conversation

ianatha
Copy link

@ianatha ianatha commented Apr 1, 2022

see issue #704

@ericgio ericgio changed the base branch from main to actions April 1, 2022 17:40
@ericgio
Copy link
Owner

ericgio commented Apr 1, 2022

Hey @ianatha, thanks for the PR! I'm not sure why the CI actions aren't running against your PR here, but it looks like this change is triggering a bunch of type errors, which I see in your fork.

@ianatha
Copy link
Author

ianatha commented Apr 1, 2022

Because the action is trying to publish a package from a non-main branch.

@ericgio
Copy link
Owner

ericgio commented Apr 1, 2022

Because the action is trying to publish a package from a non-main branch

Got it, thanks. The change still triggers failures though.

@ianatha
Copy link
Author

ianatha commented Apr 3, 2022 via email

@ericgio ericgio deleted the branch ericgio:actions April 5, 2022 08:24
@ericgio ericgio closed this Apr 5, 2022
@ericgio
Copy link
Owner

ericgio commented Apr 5, 2022

I wanted to ensure we agreed on what the top-level type safe API looks like before cascading the parametric type down

@ianatha is there any particular part of the change you wanted to discuss or that needs my attention?

Also, I unintentionally closed the PR by deleting another branch; would you mind please re-submitting the PR? I apologize for that, and thank you for taking the time to work on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants