-
-
Notifications
You must be signed in to change notification settings - Fork 14
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 documentation and examples #20
Conversation
README.md
Outdated
``` | ||
|
||
To run the test suite, go to the project root and run: | ||
|
||
```bash | ||
$ php vendor/bin/phpunit | ||
php vendor/bin/phpunit |
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 think we use this version in other projects, should be apply the same here?
php vendor/bin/phpunit | |
vendor/bin/phpunit |
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.
thank you for noticing this, should be fixed now.
98949b0
to
47b17b1
Compare
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.
Looks good so far, just noticed that examples/php.php
could also need rejection handling, similar as you already did this in examples/bash.php
& examples/docker.php
47b17b1
to
e4dd429
Compare
oops, didn't saw the |
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 for applying the requested changes, looks good to me 👍
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.
@yadaiio Thanks for looking into this, changes LGTM!
This pull request builds on top of: clue/reactphp-sqlite#43 and clue/reactphp-tar#20.