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

Doc - adds the JS HTTP Client and links to repos for the supported versions #4735

Open
wants to merge 6 commits into
base: master
from

Conversation

@prashant-shahi
Copy link
Member

prashant-shahi commented Feb 5, 2020

Fixes #4650

Signed-off-by: Prashant Shahi prashant@dgraph.io


This change is Reviewable

Docs Preview: Dgraph Preview

Signed-off-by: Prashant Shahi <prashant@dgraph.io>
@sleto-it

This comment has been minimized.

Copy link
Member

sleto-it commented Feb 5, 2020

Hi @prashant-shahi , thanks for opening this

I am a bit concerned about the fact that we write "fully supports Dgraph v1.0.x" as 1.0 is old. I believe (and I understand this is another issue) we can do this way:

  • we add a section "Supported Version" in each Readme file of the driver repos, where we create a table similar to what we have here https://github.com/dgraph-io/dgraph-js#quickstart
  • then we add a link to those tables and remove those "fully supports Dgraph v1.0.x"

This will allow us to change the supportability info in the repos, and not need to touch the this doc every time. WDYT?

We can do it in another PR if needed

Hope this helps,

@sleto-it

This comment has been minimized.

@prashant-shahi

This comment has been minimized.

Copy link
Member Author

prashant-shahi commented Feb 5, 2020

Hello @sleto-it , your raised PRs looks good.

The client versions 2.X.Y of dgraph-js and dgo should work well with Dgraph version >= 1.1.X

@sleto-it sleto-it changed the title Adding JS HTTP Client to docs Doc - adds the JS HTTP Client and links to repos for the supported versions Feb 5, 2020
Copy link
Member

sleto-it left a comment

Thanks! Please do not merge until all the external driver PRs (mentioned in a comment) get merged

Copy link
Member

martinmr left a comment

:lgtm:

Should there be a similar page for the C# client?

Reviewed 1 of 1 files at r2.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @danielmai and @MichaelJCompton)

@sleto-it

This comment has been minimized.

Copy link
Member

sleto-it commented Feb 6, 2020

@martinmr thanks for the review - yes, and we can do the changes in the PR where we cite dot net as official driver (#4568) - there will be some conflicts there when we merge this, but can be resolved

thanks,

@prashant-shahi

This comment has been minimized.

Copy link
Member Author

prashant-shahi commented Feb 7, 2020

@sleto-it Thanks for all of the work on creating multiple PRs in each of the clients.

Just have a query on the PR raised for the C# client.
I believed that we were planning on officially maintaining the repo in Dgraph instead of the main repo present in MichaelJCompton.

Just wanted to know if it was intentional to create PR in the latter.

@sleto-it

This comment has been minimized.

Copy link
Member

sleto-it commented Feb 8, 2020

Thanks - yeah, opened the PR in the wrong repo by mistake, but the changes are in a branch in the correct repo - https://github.com/dgraph-io/Dgraph-dotnet/tree/santo/supported-versions

We will need to open a new PR in the correct repo

@sleto-it

This comment has been minimized.

Copy link
Member

sleto-it commented Feb 13, 2020

@prashant-shahi the dot-net PR has now been opened in the correct repo and merged (dgraph-io/Dgraph-dotnet#2)

@sleto-it

This comment has been minimized.

Copy link
Member

sleto-it commented Feb 14, 2020

Status: all related PRs merged but the java one, which needs some more internal discussion / work

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

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.