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

#1570: add executionResult field to SmallRyeContext #1575

Merged
merged 4 commits into from
Sep 27, 2022
Merged

#1570: add executionResult field to SmallRyeContext #1575

merged 4 commits into from
Sep 27, 2022

Conversation

ikenox
Copy link

@ikenox ikenox commented Sep 26, 2022

Fixes #1570

phillip-kruger
phillip-kruger requested changes Sep 26, 2022
View reviewed changes
Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

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

LGTM, one change request

server/implementation/src/main/java/io/smallrye/graphql/execution/context/SmallRyeContext.java Outdated Show resolved Hide resolved
phillip-kruger
phillip-kruger previously approved these changes Sep 27, 2022
View reviewed changes
Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

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

LGTM ! Thanks

@phillip-kruger phillip-kruger added this to the 2.0.0 milestone Sep 27, 2022
@phillip-kruger phillip-kruger added enhancement New feature or request back-port This needs to back-port to the 2.0.x branch labels Sep 27, 2022
@phillip-kruger
Copy link
Member

phillip-kruger commented Sep 27, 2022 •
edited
Loading

Something still off. Do a full build locally, then commit: Error: Failed to execute goal net.revelc.code:impsort-maven-plugin:1.6.2:check (default-cli) on project smallrye-graphql: Imports are not sorted in /home/runner/work/smallrye-graphql/smallrye-graphql/server/implementation/src/main/java/io/smallrye/graphql/execution/context/SmallRyeContext.java -> [Help 1]

@ikenox
Copy link
Author

ikenox commented Sep 27, 2022

@phillip-kruger
Sorry for failing CI repeatedly.
I've successfully done a build locally, and CI seems to be passed on my forked repository on e15deca (link).

@phillip-kruger
Copy link
Member

Thanks !
Let see what CI does now.

@phillip-kruger phillip-kruger merged commit 3537d16 into smallrye:main Sep 27, 2022
@phillip-kruger
Copy link
Member

@jmartisk this can be backported to 1.8.x

@jmartisk jmartisk mentioned this pull request Sep 27, 2022
Merged
@jmartisk
Copy link
Member

@phillip-kruger, sent a PR for that: #1576

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillip-kruger phillip-kruger phillip-kruger left review comments

Assignees
No one assigned
Labels
back-port This needs to back-port to the 2.0.x branch enhancement New feature or request
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.

Provide a way to access an ExecutionResult instance
3 participants
@ikenox @phillip-kruger @jmartisk