-
Notifications
You must be signed in to change notification settings - Fork 15
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
v23.0.0 Update & Clean PR #2: Dgraph Client Refactor #33
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good. I am happy to see that you have cleaned up the repo too, added comments and got it closer to dgo. Good work!
I was able to merge the first PR, please do a rebase and we will review this PR. |
Rebase complete @mangalaman93 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a few more comments, this looks good to me. @all-seeing-code @MichelDiz please review & approve once my comments are addressed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one more comment, then we are good to merge.
Tests fail right now. Is that expected? |
@all-seeing-code yeah, next PR will fix it |
Most of the changed files contain simple style, formatting, and naming cleanup.
Functional changes are mainly related to aligning this client to the dgo client and are concentrated in these classes/interfaces:
DgraphCloudChannel
Create
method implements the dgoDialCloud
logic.DgraphClient
IDgraphClient
DgraphClient
class directly.Transaction
Do
method similar to the dgo implementation.ITransaction
Transaction
class directly.IQuery
Transaction
class is hidden behind this interface when creating it withclient.NewReadOnlyTransaction
RequestBuilder
MutationBuilder
README.md is also heavily rewritten to match the dgo readme as closely as possible.
Note: The tests will fail until PR #34 is merged.