Resolve some todos in async-await branch by nWacky · Pull Request #452 · graphql-rust/juniper

nWacky · 2019-11-04T16:26:47Z

This PR resolves some todos in async-await branch.

Changelog:

Resolve RFC 2565 related todos
Remove __juniper_extract_generic macro
Add resolve_into_type_async to GraphQLTypeAsync for asynchronous fragments resolving
Update GraphQLTypeAsync with async-trait, update macros
Resolve "better error message with field/type name" todo, update other panics with query name (query name retrieved by calling self::name(info))
Rebase onto master
Format

…panics with query name

…nstrumentisto/juniper into async-await-resolve-some-todos

LegNeato

Thanks again for so much work! Some small tweaks / questions. Also, can you rebase on async-await again?

LegNeato · 2019-11-05T06:00:45Z

-juniper_warp = { git = "https://github.com/graphql-rust/juniper", branch = "async-await", features = ["async"] }
+#juniper_codegen = { git = "https://github.com/graphql-rust/juniper", branch = "async-await", features = ["async"] }
+#juniper = { git = "https://github.com/graphql-rust/juniper", branch = "async-await", features = ["async"] }
+#juniper_warp = { git = "https://github.com/graphql-rust/juniper", branch = "async-await", features = ["async"] }


Nit: Remove these

LegNeato · 2019-11-05T06:03:16Z

-    //     #[doc = "and more details"]
-    //     arg: i32,
-    // ) -> i32 { 0 }
+    fn attr_arg_descr(arg: i32) -> i32 {


This looks wrong? Also, we have #441 outstanding

Got it.
I put it back to todos until #441 is resolved

LegNeato · 2019-11-05T06:04:12Z

+}
+enum ResolversWithTrailingComma {
+    Concrete(Concrete),
+}


Why were these added back?

Oops, my mistake

I don't see this fixed? Is this a bad merge?

…panics with query name

…nstrumentisto/juniper into async-await-resolve-some-todos # Conflicts: # examples/warp_async/Cargo.toml # juniper/src/macros/tests/args.rs # juniper_codegen/src/util.rs [skip ci]

LegNeato · 2019-11-06T01:59:27Z

 }

+/*
+ * FIXME: make this work again


Something is wonky with the merge or you haven't rebased on latest async-await as this undoes a change we want.

LegNeato · 2019-11-06T02:00:59Z

    }

+    /*
+     * FIXME: make this work again


This looks like you need to rebase or a bad merge?

LegNeato · 2019-11-06T02:01:44Z

+}
+enum ResolversWithTrailingComma {
+    Concrete(Concrete),
+}


I don't see this fixed? Is this a bad merge?

LegNeato

I'm seeing some weirdness where stuff in the async-await branch is undone. Not sure if you just need to rebase or there is something else going on.

…nstrumentisto/juniper into async-await-resolve-some-todos # Conflicts: # juniper/src/macros/tests/args.rs [skip ci]

nWacky · 2019-11-06T09:06:11Z

Yes, git messed everything up while merging. Sorry for that.
Now everything should be good

theduke · 2019-11-13T12:46:41Z

I'm sorry I didn't see this before, but I'm not actually on board with using async-trait.

It's a nice library, but it just adds a bit of syntax sugar, and it very well might differ from the semantics of compiler support, which isn't on the near-horizon. Also 99% of the code will be auto-generated by the macros, so we want to avoid that extra dependency.

theduke · 2019-11-13T12:59:39Z

Could you handle reverting, @nWacky ?

nWacky · 2019-11-13T13:07:51Z

@theduke

Yes, I'll remove async-trait today. Juniper's macros don't depend on async-trait, I wrote all lifetimes by hand

theduke · 2019-11-13T13:12:40Z

Thanks. I think we will want to merge the async branch in the next few days to avoid extra churn.

I also have some bigger refactorings upcoming that would affect subscriptions, we might want to chat about that on Gitter or somewhere.

nWacky · 2019-11-13T20:47:51Z

Yeah, that would be great

LegNeato · 2019-11-14T05:07:57Z

@theduke I converted a project at work to async_trait and it made the implementations much more readable as everything didn't have to have Box and Pin everywhere. It also let me clean up some futures::future::Either use as well. But I guess I don't feel strongly enough to keep it.

theduke · 2019-11-14T07:40:40Z

I have a type system change which gets rid of GraphlQLTypeAsync that makes this impossible.

Not 100% sure yet if it will work out, but if it does it should be ready soon.

andy128k and others added 24 commits October 23, 2019 18:04

Bubble up scalar error (#434)

dd424f3

Update CHANGELOG.md

9be274a

Make EmptyMutation Send

8628ddd

Update CHANGELOG.md

6b7977b

Release juniper_codegen 0.14.1

252f7b4

Release juniper 0.14.1

c434bdd

Release juniper_hyper 0.5.1

012c0b1

Release juniper_iron 0.6.1

0421f9a

Release juniper_rocket 0.5.1

7c98eaf

Release juniper_warp 0.5.1

91a9752

Resolve RFC 2565 related todos

bd3d289

Remove __juniper_extract_generic macro

0f4e998

Start adding async-trait to GraphQLTypeAsync

13bbbe2

Merge remote-tracking branch 'upstream/async-await' into async-await

563e44c

Add resolve_into_type_async

737c4c7

Refactor GraphQLTypeAsync to use async-trait

9e2a63a

Remove useless todo

9478b6c

Resolve better error message with field/type name todo, update other …

91dc55b

…panics with query name

Format

afd92cd

Rebase onto master

2404b81

Merge branch 'async-await-resolve-some-todos' of https://github.com/i…

09d1077

…nstrumentisto/juniper into async-await-resolve-some-todos

Format

503bb63

Merge import in async_await

e2ebaee

Format

3e5e2ae

LegNeato requested changes Nov 5, 2019

View reviewed changes

andy128k and others added 5 commits November 5, 2019 11:14

Bubble up scalar error (#434)

779208e

Update CHANGELOG.md

7230efc

Make EmptyMutation Send

665c3d2

Update CHANGELOG.md

bb99076

Release juniper_codegen 0.14.1

7b18bd2

nWacky added 11 commits November 5, 2019 11:14

Refactor GraphQLTypeAsync to use async-trait

52c3e28

Remove useless todo

b1970ae

Resolve better error message with field/type name todo, update other …

00dd1dc

…panics with query name

Format

e3c12e3

Rebase onto master

dbcaf30

Format

554ba34

Merge import in async_await

9506272

Format

7135c07

Fix cargo.toml in warp_async

ec76bf5

Comment out attr_arg_descr and attr_arg_descr_collapse tests

99c0d26

Merge branch 'async-await-resolve-some-todos' of https://github.com/i…

e151026

…nstrumentisto/juniper into async-await-resolve-some-todos # Conflicts: # examples/warp_async/Cargo.toml # juniper/src/macros/tests/args.rs # juniper_codegen/src/util.rs [skip ci]

nWacky requested a review from LegNeato November 5, 2019 16:30

LegNeato reviewed Nov 6, 2019

View reviewed changes

LegNeato requested changes Nov 6, 2019

View reviewed changes

nWacky added 4 commits November 6, 2019 11:21

Rebase onto async-await

4834349

Rebase onto master

09d9513

Merge branch 'async-await-resolve-some-todos' of https://github.com/i…

e344f1c

…nstrumentisto/juniper into async-await-resolve-some-todos # Conflicts: # juniper/src/macros/tests/args.rs [skip ci]

Fix bad merge [skip ci]

778606c

LegNeato approved these changes Nov 7, 2019

View reviewed changes

LegNeato merged commit b133a0f into graphql-rust:async-await Nov 7, 2019

tyranron deleted the async-await-resolve-some-todos branch November 7, 2019 06:40

nWacky mentioned this pull request Nov 13, 2019

Remove async trait from async-await #460

Merged

Conversation

nWacky commented Nov 4, 2019

Uh oh!

LegNeato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nWacky Nov 5, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LegNeato left a comment

Choose a reason for hiding this comment

Uh oh!

nWacky commented Nov 6, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theduke commented Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

theduke commented Nov 13, 2019

Uh oh!

nWacky commented Nov 13, 2019

Uh oh!

theduke commented Nov 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nWacky commented Nov 13, 2019

Uh oh!

LegNeato commented Nov 14, 2019

Uh oh!

theduke commented Nov 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

nWacky Nov 5, 2019 •

edited

Loading

nWacky commented Nov 6, 2019 •

edited

Loading

theduke commented Nov 13, 2019 •

edited

Loading

theduke commented Nov 13, 2019 •

edited

Loading

theduke commented Nov 14, 2019 •

edited

Loading