-
Notifications
You must be signed in to change notification settings - Fork 63
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
Update Java v1.8.x support in README.md #146
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.
Review the preview file and make sure the Markdown renders as expected: https://github.com/dgraph-io/dgraph4j/blob/2ef430d559cfbfecd285d8ac8bc0f65003a0b5f5/README.md
Otherwise, I have a couple of comments on phrasing and the version table.
Reviewable status: 0 of 1 files reviewed, 10 unresolved discussions (waiting on @abhimanyusinghgaur and @danielmai)
README.md, line 72 at r1 (raw file):
don't want to use
aren't using
README.md, line 72 at r1 (raw file):
above version table
"the above version table"
README.md, line 74 at r1 (raw file):
you want to use
Simplify to "you're using"
README.md, line 81 at r1 (raw file):
```xml
This needs a newline before these backticks to be rendered correctly in Markdown.
README.md, line 87 at r1 (raw file):
```xml
This needs a newline after these backticks to be rendered correctly in Markdown.
README.md, line 89 at r1 (raw file):
```groovy
This needs a newline before these backticks to be rendered correctly in Markdown.
README.md, line 91 at r1 (raw file):
This needs a newline after these backticks to be rendered correctly in Markdown.
README.md, line 92 at r1 (raw file):
Following
"The following"
README.md, line 94 at r1 (raw file):
grpc-netty
This table should also list out the corresponding netty-tcnative-boringssl-static
versions too. Otherwise, it's not obvious without looking at the gRPC docs where you get 2.0.26-Final
from.
README.md, line 102 at r1 (raw file):
So, for example
Add a line between the table and this paragraph so it renders correctly in Markdown (see the rendered version here: https://github.com/dgraph-io/dgraph4j/blob/2ef430d559cfbfecd285d8ac8bc0f65003a0b5f5/README.md)
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.
I guess, I can't trust my IDE anymore with markdown files, It was rendering it differently than what GitHub was rendering.
Reviewable status: 0 of 1 files reviewed, 10 unresolved discussions (waiting on @danielmai)
README.md, line 72 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
don't want to use
aren't using
Done.
README.md, line 72 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
above version table
"the above version table"
Done.
README.md, line 74 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
you want to use
Simplify to "you're using"
Done.
README.md, line 81 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
```xml
This needs a newline before these backticks to be rendered correctly in Markdown.
Done.
README.md, line 87 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
```xml
This needs a newline after these backticks to be rendered correctly in Markdown.
Done.
README.md, line 89 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
```groovy
This needs a newline before these backticks to be rendered correctly in Markdown.
Done.
README.md, line 91 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
This needs a newline after these backticks to be rendered correctly in Markdown.
Done.
README.md, line 92 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
Following
"The following"
Done.
README.md, line 94 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
grpc-netty
This table should also list out the corresponding
netty-tcnative-boringssl-static
versions too. Otherwise, it's not obvious without looking at the gRPC docs where you get2.0.26-Final
from.
Done.
README.md, line 102 at r1 (raw file):
Previously, danielmai (Daniel Mai) wrote…
So, for example
Add a line between the table and this paragraph so it renders correctly in Markdown (see the rendered version here: https://github.com/dgraph-io/dgraph4j/blob/2ef430d559cfbfecd285d8ac8bc0f65003a0b5f5/README.md)
Done.
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.
Ah. Yeah, Markdown is not consistent. There should be an extension to give you the same rendering that GitHub would show.
Reviewed 1 of 1 files at r2.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @danielmai)
Fixes #144.
Not adding a test because of following reasons:
dgraph4j
as it will enforce it for all java versions, which in not what we want.DgraphJavaSample
, as people will be looking at it irrespective of java version they use. So, I think its better for it to stay as is. Just the documentation update should be enough to guide people who face problems with Java 1.8.xThis change is