-
-
Notifications
You must be signed in to change notification settings - Fork 744
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
Fixed StrawberryShake result has empty Extensions Issue #5967
Fixed StrawberryShake result has empty Extensions Issue #5967
Conversation
418bda9
to
1d6052b
Compare
Hey there, thank you for contributing. Can you add some integration tests so that we can see that this works? |
1d6052b
to
9fcd086
Compare
@michaelstaib Not sure about integration tests, but I added a simple unit test that does some basic assertions. From what I could understand, objects of Also, I've just empirically confirmed that the fix in fact works. |
9fcd086
to
16da171
Compare
@michaelstaib What would be my (or some else's) next steps on this? |
@michaelstaib What else is needed? Can this fix make into 13.1? How can we help in addition to the @anton-gogolev fix and unit test? The |
Yes |
We will add it to 13.1 |
Should be merged after the weekend and available as preview |
@michaelstaib Thank you, it's a great news!!! @anton-gogolev many thanks for the fix! |
@michaelstaib A friendly ping here. Is anything required here from me? |
@michaelstaib https://github.com/ChilliCream/graphql-platform/releases/tag/13.1.0-preview.5 is out with this fix promised to be included
It's all right if it is not forgotten as such and will get into 13.1. But we worry a little... and we do not have a chance to test a preview, who knows, the merge may bring surprises. |
@michaelstaib could you please merge this PR so that it is included in the preview and we can test? We promise to test and report whatever the outcome is. You should be interested in any outcome and our feedback, right? |
@michaelstaib Sorry to nag ... can this be merged please? Who else can review? This is holding up @anton-gogolev and our Altium365 team... |
Its merged and will be in the next build. |
This is my naïve attempt to fix #4956.
There are no meaningful tests for this fix; I hacked around existing one to get this thing to kinda-sorta work to fit my use-case. I'm all ears and open to suggestions as to how improve this PR.