join__directive support in extract_subgraphs_from_supergraph#5720
join__directive support in extract_subgraphs_from_supergraph#5720lennyburdette merged 3 commits intodevfrom
Conversation
|
@lennyburdette, please consider creating a changeset entry in |
|
CI performance tests
|
lrlna
left a comment
There was a problem hiding this comment.
A small comment on .expect() style. Otherwise this should be good to go!
There was a problem hiding this comment.
the message of .expect() should be explaining why the Result/Option of the value should have been there (guidance from rust docs). All of these are worded in an .assert() style when it's not actually an assert. These should read something like "join__directive(graphs:) value is an enum" or something of the sort. There are a bunch of good examples in apollo-router/src/json_ext.rs with expect statements.
There was a problem hiding this comment.
can you add the todos to jira as well? we lose track of these so easily
There was a problem hiding this comment.
and some federation constants this change is mostly useless at the moment because the yet-to-be-implemented connector directives are the only ones eligible for join__directive serialization
4a19cc0 to
f045e2e
Compare
and some federation constants
this change is mostly useless at the moment because the yet-to-be-implemented connector directives are the only ones eligible for join__directive serialization
Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.
Exceptions
Note any exceptions here
Notes
Footnotes
It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this. ↩
Configuration is an important part of many changes. Where applicable please try to document configuration examples. ↩
Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions. ↩