-
Notifications
You must be signed in to change notification settings - Fork 387
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
docs: minor tweaks to contributing.md #464
Conversation
CONTRIBUTING.md
Outdated
@@ -27,8 +27,8 @@ your new repository and run the build scripts: | |||
|
|||
You need to have some supported version of Python installed and have | |||
it available as `python` in your shell. To run Zookeeper you also | |||
need a Java runtime (JRE or JDK) version 6 or 7. To run tests, you | |||
need to have the tox, Python testing tool, to be installed in your shell. | |||
need a Java runtime (JRE or JDK) version 6, 7 or 8. To run tests, you |
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.
Is this dependency on JDK/JRE 6 or 7 hardcoded anywhere? I added 8 now that it's widely used in production, but personally I'd rather remove the version numbers altogether as it's one less thing that needs updating.
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'm not sure, the dependency is whatever version Zookeeper wants. I'd imagine we could likely just say 6+ since afaik none of the newer ones are going to break old things.
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.
It makes sense to replace this with something like: "To run Zookeeper you also need a Java runtime (JRE or JDK). Please refer to the Zookeeper documentation for compatible Java versions for each Zookeeper version."
CONTRIBUTING.md
Outdated
@@ -27,8 +27,8 @@ your new repository and run the build scripts: | |||
|
|||
You need to have some supported version of Python installed and have | |||
it available as `python` in your shell. To run Zookeeper you also | |||
need a Java runtime (JRE or JDK) version 6 or 7. To run tests, you | |||
need to have the tox, Python testing tool, to be installed in your shell. | |||
need a Java runtime (JRE or JDK) version 6, 7 or 8. To run tests, you |
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'm not sure, the dependency is whatever version Zookeeper wants. I'd imagine we could likely just say 6+ since afaik none of the newer ones are going to break old things.
8ba0713
to
40b92a8
Compare
Fix readability, tense, grammar, etc.
40b92a8
to
6fd1057
Compare
Thanks for the suggested wording @hannosch, I updated the PR accordingly. This is ready to be reviewed/merged. |
Fix readability, tense, grammar, etc.