-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
README update #9488
README update #9488
Conversation
I hope you don't mind, but I took the liberty to make some changes to wording that would make things more succinct and clearer. I also made certain items links because the way they were written kind of positioned them in a way that makes more sense to link them. Please let me know if there are any problems and I'll be sure to fix them. Thank you for this opportunity. I went through the docs and you guys are doing awesome things.
Thanks for opening this pull request! This space is protected by our Code of Conduct - and we're here to help. |
Codecov Report
@@ Coverage Diff @@
## main #9488 +/- ##
=======================================
Coverage ? 81.50%
=======================================
Files ? 98
Lines ? 5938
Branches ? 0
=======================================
Hits ? 4840
Misses ? 1098
Partials ? 0 |
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.
Great work @OlaoluwaM! Looks good to me, thanks for opening this PR!
Great!! Thank you so much. So what do I do next 😅? |
Please wait for the project maintainers to review the PR. 😄 |
hey! @OlaoluwaM please remove those "✔️" by editing. Save it then later tick when the box appears |
@NishantJawla Done! |
So is there anything else I need to do 🙂 |
Just wait for a collaborator to review this PR |
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.
Tremendous!!! thanks a lot @OlaoluwaM for going through the doc and helping improve it...please let me know what you think of my suggestions. Thanks again for working on this
README.md
Outdated
@@ -184,18 +181,19 @@ Adding `bundle exec` ensures you're using the version of passenger you just inst | |||
|
|||
## Reply-by-email | |||
|
|||
Public Lab now supports reply by email to comment feature. For more details regarding it go to the [email documentation](https://github.com/publiclab/plots2/blob/main/doc/EMAIL.md) | |||
Public Lab now supports reply by email to comment features. For more details, go to the [email documentation](https://github.com/publiclab/plots2/blob/main/doc/EMAIL.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 believe we would like to keep it as singular because the "reply by email comment" is one feature..what do you think?
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.
Ohh, that makes a lot of sense. If that's the case wouldn't be better to string them together like "reply-by-email-to-comment" feature? Or put them in quotes so anyone reading knows that they go together
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 agree, the quotes would make the phrase better. Thanks
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.
How do I update the PR?
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.
Update the changes from here https://github.com/OlaoluwaM/plots2/tree/patch-1 and commit, the commit will appear over here too. thanks
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.
Done!
For the last bit (change), a colon would work, but the problem is the "and a wiki" part. Colons are primarily used to tack on more information or an explanation onto the end of an independent clause. You can check out this site for more info However, I could be getting something wrong, the "group research blog," is it both the "research notes" and "wiki" or just "research notes"? If it is the former then yes, a colon would be appropriate. If it is the latter, then the "what we call 'research notes'" part is more of an aside than a component of the sentence. I say aside because it exists to add more information, not necessarily to make the sentence make sense; if we were to remove it no one would notice and the sentence would still convey meaning. Therefore, I tried to demarcate it that way with the dashes, but commas would work equally as well. In fact, commas would have been better since they are more common. |
Code Climate has analyzed commit a49c402 and detected 0 issues on this pull request. View more on Code Climate. |
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.
thanks again @OlaoluwaM 🎉
Congrats on merging your first pull request! 🙌🎉⚡️ Help others take their first stepNow that you've merged your first pull request, you're the perfect person to help someone else out with this challenging first step. 🙌 Try looking at this list of `first-timers-only` issues, and see if someone else is waiting for feedback, or even stuck! 😕 People often get stuck at the same steps, so you might be able to help someone get unstuck, or help lead them to some documentation that'd help. Reach out and be encouraging and friendly! 😄 🎉 Read about how to help support another newcomer here, or find other ways to offer mutual support here. |
* README update I hope you don't mind, but I took the liberty to make some changes to wording that would make things more succinct and clearer. I also made certain items links because the way they were written kind of positioned them in a way that makes more sense to link them. Please let me know if there are any problems and I'll be sure to fix them. Thank you for this opportunity. I went through the docs and you guys are doing awesome things. * fix(docs): quoted and made word singular
I hope you don't mind, but I took the liberty to make some changes to wording that would make things more succinct and clearer. I also made certain items links because the way they were written kind of positioned them in a way that makes more sense to link them. Please let me know if there are any problems and I'll be sure to fix them. Thank you for this opportunity. I went through the docs and you guys are doing awesome things.
Fixes #9479 (<=== Add issue number here)
Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!
rake test
@publiclab/reviewers
for help, in a comment belowIf tests do fail, click on the red
X
to learn why by reading the logs.Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software
Thanks!