-
Notifications
You must be signed in to change notification settings - Fork 11
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
Phpunit integration && PHP 7 update #4
base: trunk
Are you sure you want to change the base?
Conversation
Also, if this work fits your expectations, would you please enable this repo in Travis for continuous integration? |
@@ -2,3 +2,6 @@ | |||
composer.lock | |||
composer.phar | |||
vendor | |||
tests/coverage | |||
.phpunit.result.cache | |||
clover.xml |
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.
These should have went into your global .gitignore.
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.
tests/coverage and clover.xml are specified in the phpunit.xml. Are you sure they should be global?
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 have .phpunit.result.cache
in pretty much in every other project. Two others are not belong here in the first place. E.g. you can put them into a build
folder, then ignore it globally. Or add a local .gitignore
with a *
in that folder. But I'm just nitpicking here. Great job!
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 with you concerning .phpunit.result.cache
but for the 2 others adding a dedicated folder which itself is ignored looks just as adding out of scope complexity. It could be necessary, if ever a full build process had to be configured, but later, when the build
folder would gather more stuff. And taking this in consideration here seems much less important than considering coverage for example.
If you have an example of a build process applied to other Pear packages, I would follow it gladly. Is it the case?
a8d3b8c
to
8bc1d1b
Compare
Moreover, would you also enable this repo on https://codecov.io (or any coverage saas) so I could enable it in Travis (passing tests means nothing without coverage)? |
Hello, your work seems to still be the best solution for Apache configuration parsing in PHP. As I try to keep safe libs in my projects, I integrated PHPUnit and updated your lib for PHP 7.
I still have one failing test: bug11435-inifile.phpt
But I'm not sure of what I have to do. I need a little help for this case.
This PR could seem big in appearance but is quit simple in fact. I just moved all the old test files to a dedicated directory. All the real modifications to the "business" code are gathered in the first commit: 1a2c2fc
Thanks in advance for your time