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.

Sign up for GitHub

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

Jump to bottom

feat(all): Add new GraphQL scalar type Int8 #4511

Merged
merged 1 commit into from
Jun 4, 2023
Merged

feat(all): Add new GraphQL scalar type Int8 #4511

merged 1 commit into from
Jun 4, 2023

Conversation

dotansimha
Copy link
Contributor

@dotansimha dotansimha commented Apr 2, 2023 •
edited
Loading

TODO

@dotansimha dotansimha force-pushed the int8-scalar branch 2 times, most recently from f41e83a to 7000fc3 Compare April 3, 2023 15:48
@dotansimha dotansimha force-pushed the int8-scalar branch 7 times, most recently from 3ccd62a to 87e1dca Compare April 17, 2023 09:28
@dotansimha dotansimha requested a review from lutter April 17, 2023 09:37
@dotansimha dotansimha marked this pull request as ready for review April 17, 2023 09:37
@dotansimha dotansimha force-pushed the int8-scalar branch 2 times, most recently from 865a9bc to b0bb9fa Compare April 17, 2023 11:28
@dotansimha dotansimha mentioned this pull request Apr 17, 2023
Merged
@dotansimha dotansimha changed the title New GraphQL scalar: Int8 feat(graph, graphql, runtime, store): new GraphQL scalar: Int8 (rust: i64, pg: bigint) Apr 17, 2023
lutter
lutter approved these changes Apr 17, 2023
View reviewed changes
Copy link
Collaborator

@lutter lutter left a comment

Choose a reason for hiding this comment

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

Nice! Looks great!

graphql/src/schema/meta.graphql Outdated Show resolved Hide resolved
graphql/src/values/coercion.rs Show resolved Hide resolved
@dotansimha dotansimha mentioned this pull request Apr 18, 2023
Open
@dotansimha dotansimha force-pushed the int8-scalar branch 6 times, most recently from 4af3fc0 to cf25a15 Compare May 31, 2023 11:29
lutter
lutter approved these changes Jun 1, 2023
View reviewed changes
Copy link
Collaborator

@lutter lutter left a comment

Choose a reason for hiding this comment

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

This looks great!

Just change the unwrap before committing. Also, for the commit message, something like

all: Add new scalar type Int8

would be more in line with what we've been doing

But with these two touchups, this is good to merge.

graph/src/schema/meta.graphql Outdated Show resolved Hide resolved
store/postgres/src/relational_queries.rs Outdated Show resolved Hide resolved
@dotansimha dotansimha changed the title feat(graph, graphql, runtime, store): new GraphQL scalar: Int8 (rust: i64, pg: bigint) feat(all): Add new scalar type Int8 Jun 4, 2023
@dotansimha dotansimha changed the title feat(all): Add new scalar type Int8 feat(all): Add new GraphQL scalar type Int8 Jun 4, 2023
@dotansimha
Copy link
Contributor Author

Thanks @lutter . Fixed the unwrap issue and the other things you mentioned. I'll merge when CI clears 🚀

@dotansimha dotansimha merged commit dc4def8 into master Jun 4, 2023
@dotansimha dotansimha deleted the int8-scalar branch June 4, 2023 07:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lutter lutter lutter approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dotansimha @lutter