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.

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

Rework the API expressed by DgsExecutionResult #1298

Merged
merged 1 commit into from Oct 29, 2022

Conversation

berngp
Copy link
Contributor

@berngp berngp commented Oct 27, 2022

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Changes in this PR

  • We are adopting a Builder pattern instead of depending on Kotlin delegation for the ExecutionResult. This was done to expose a cleaner API for Java Consumers.
  • We are also moving the DgsExecutionResult to not be internal, since at is clear from the MyInstrumentation example developers will be using it explicitly.

@berngp berngp force-pushed the feature/rework-DgsExecutionResult-API branch from 40bb395 to f861fc1 Compare October 27, 2022 21:07
@berngp berngp force-pushed the feature/rework-DgsExecutionResult-API branch from f861fc1 to fa6ebcb Compare October 27, 2022 22:07
* We are adopting a Builder pattern instead of depending on Kotlin delegation for
  the ExecutionResult. This was done to expose a cleaner API for Java Consumers.
* We are also moving the `DgsExecutionResult` to not be _internal_, since at is clear from the `MyInstrumentation`
  example developers will be using it explicitly.
@berngp berngp force-pushed the feature/rework-DgsExecutionResult-API branch from fa6ebcb to 27401d3 Compare October 28, 2022 23:43
@berngp berngp merged commit 87b3501 into master Oct 29, 2022
@berngp berngp deleted the feature/rework-DgsExecutionResult-API branch October 29, 2022 00:36
@antholeole
Copy link
Contributor

Ah, I didn't consider these things, sorry! These are good changes, thanks for the refactor.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants