Fixed some typos in README.md file #167
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #167 +/- ##
============================================
+ Coverage 76.64% 76.69% +0.04%
- Complexity 289 290 +1
============================================
Files 42 42
Lines 989 991 +2
Branches 43 43
============================================
+ Hits 758 760 +2
Misses 206 206
Partials 25 25 ☔ View full report in Codecov by Sentry. |
felix-seifert
left a comment
felix-seifert
left a comment
There was a problem hiding this comment.
It is nice that you help us to maintain the documentation. Often, people ignore the quality of the documentation or just forget some details. For landing your PR, please have a look at the comments I left and apply the necessary changes to your PR. Either revert the commented code or improve it slightly.
README.md
Outdated
| A small Java library for talking to GitHub/GitHub Enterprise and interacting with projects. | ||
|
|
||
| It supports authentication via simple access tokens, JWT endpoints and GitHub Apps (via private key). | ||
| It supports authentication via simple access tokens, JWT endpoints, and GitHub Apps (via private key). |
There was a problem hiding this comment.
Usually, the comma you added is not needed (I am not sure whether correct grammar even accepts this comma). I only add this comma if it increases the readability if I list long items. Think of very long item, another very long item, and a third long item. In the above case, I would not add it.
README.md
Outdated
| It is also very light on GitHub, doing as few requests as necessary. | ||
|
|
||
| This library is maintained by @spotify/gjc-maintainers. If you have any questions, issues or need a | ||
| This library is maintained by @spotify/gjc-maintainers. If you have any questions, or issues or need a |
There was a problem hiding this comment.
This additional or can be debated because questions and issues are for the verb have whereas the third item need a review has its own verb. If we actually decided to add this or, the used commas are confusing for me. I would also suggest to not introduce this change.
README.md
Outdated
| - Checkout on to master locally and pull the latest changes | ||
| - Run `mvn release:prepare`, this will generate 2 commits that will bump the version of the github-java-client | ||
| - Push these changes to master | ||
| - Push these changes to the master |
There was a problem hiding this comment.
We either talk about pushing changes to master or to the master branch. You can consider changing the whole list to either one or the other option to keep consistency.
There was a problem hiding this comment.
This is still very confusing... Can you please address this? Probably just revert your change.
|
@felix-seifert Check now this PR |
README.md
Outdated
| - Checkout on to master locally and pull the latest changes | ||
| - Run `mvn release:prepare`, this will generate 2 commits that will bump the version of the github-java-client | ||
| - Push these changes to master | ||
| - Push these changes to the master |
There was a problem hiding this comment.
This is still very confusing... Can you please address this? Probably just revert your change.
|
@felix-seifert See now |
2 participants
I Fixed some Grammar and phrases in
README.mdfile.please go through that.