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

introduce @effect/schema #1241

Merged
merged 5 commits into from
May 28, 2024
Merged

introduce @effect/schema #1241

merged 5 commits into from
May 28, 2024

Conversation

KhraksMamtsov
Copy link
Contributor

No description provided.

Copy link
Owner

@moltar moltar left a comment

Choose a reason for hiding this comment

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

Also, please add to the README (a-z).

package.json Outdated Show resolved Hide resolved
@moltar moltar requested a review from hoeck May 23, 2024 21:25
@moltar
Copy link
Owner

moltar commented May 27, 2024

screenshot-20240527T145638-TMrKagW5@2x

Do we need the result const?

@KhraksMamtsov
Copy link
Contributor Author

Nope

@KhraksMamtsov
Copy link
Contributor Author

But otherwise it shows an error

Assertions require every name in the call target to be declared with an explicit type annotation.ts(2775)

effect-schema.ts(67, 9): 'asserts' needs an explicit type annotation.

@moltar
Copy link
Owner

moltar commented May 27, 2024

But otherwise it shows an error

Assertions require every name in the call target to be declared with an explicit type annotation.ts(2775)

effect-schema.ts(67, 9): 'asserts' needs an explicit type annotation.

Where do you seed that?

PR build seems to pass.

@KhraksMamtsov
Copy link
Contributor Author

I fixed this warning)
when can I expect a merge?

@moltar moltar merged commit fa1c1eb into moltar:master May 28, 2024
6 checks passed
@moltar
Copy link
Owner

moltar commented May 28, 2024

I was waiting for @hoeck to review. But maybe he's busy. I'll merge for now.

@hoeck Please do a reverse review when/if you have time.

@KhraksMamtsov
Copy link
Contributor Author

Thank you

@hoeck
Copy link
Collaborator

hoeck commented May 28, 2024

@moltar had already a look at it and was waiting for the explanation for the warning that got fixed.

Looks all good to me 👍

Thanks @KhraksMamtsov

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.

3 participants