-
Notifications
You must be signed in to change notification settings - Fork 79
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
Improve readme and --help #473
Conversation
Hi yes, I am not sure how to review this. I dunno how people do code review on prose. Stylistically I have a preference for avoiding the passive voice. (So "it is recommended" would become "we recommend".) |
If you like it overall, feel free to make whatever tweaks you like and push them to my branch if you want. For broader changes, feel free to suggest things like “remove this section”, “this is good but that is more important so it should be more prominent” or “I see what problems you tried to solve but I’d like to phrase things a completely different way”. Or push big changes to my branch if you feel like it. Or another branch based on mine. Or review any way you like. I don’t mind. 😄 Edit: Also, if “please avoid passive voice” is your only piece of feedback I can try to fix it myself too. |
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 put some comments. A lot are very minor and almost all are subjective so feel free to push back.
Co-authored-by: Harry Sarson <[email protected]>
I am happy, will just wait and see if anyone else wants to take a look before we merge. |
Conflicts: lib/elm-test.js
--help
.--help
.PR for elm-explorations/test: elm-explorations/test#160
We should also update the repo description because it currently says: