-
Notifications
You must be signed in to change notification settings - Fork 3
[v2.0] [BC Break] Rewrite library with ConsoleHelper + tests #8
[v2.0] [BC Break] Rewrite library with ConsoleHelper + tests #8
Conversation
Green! :) Ready for review @weierophinney |
Script is changed to: $ vendor/bin/composer autoloading <command> [options] modulename Commands: - help - display help/usage message - enable - enable composer autoloading for the module - disable - disbale composer autoloading for the module Options are the same as previously: - --help|-h - display help/usage message - --composer|-c - composer binary path; defaults composer - --type|-t - module autoloading type to use - --modules-path|-p - path to directory with modules Added tests for the whole library and added coveralls configuration.
- svg travis build badge pointing master branch - added coverage status badge
Variable $publicDir is now private
Redirect stderr to stdout 2>&1 Works also on Windows.
AbstractCommand now has public method process instead of __invoke. Command have to implement method execute, which is called in process.
This tests mock "system" method so it should be run in separate process.
Run test on PHP 5.6/7.0/7.1 with lowest/locked/latest dependencies.
Tests vere failing on PHP7.0/7.1 with lowest dependencies
Without this annotation these tests on travis are failing. On local I can't observe this issue.
@weierophinney rebased |
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.
Two minor nitpicks; overall, love this. I'll take care of the requested changes on merge.
@@ -8,7 +8,7 @@ | |||
"console", | |||
"framework", | |||
"zendframework", | |||
"zf2" | |||
"zf3" |
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'd argue we should drop the version-specific tags, and just stick with zendframework
.
{ | ||
$this->path = $path; | ||
$this->composerJsonFile = sprintf('%s/composer.json', $path); | ||
$this->command = (string)$command; |
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.
Should be a space following the )
; I thought that was setup in the zend-coding-standard?
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.
Yeh, there should be space. It is not in version 1.0.0 of zend-coding-standard unfortunately but it is coming with big upgrade ;-)
[v2.0] [BC Break] Rewrite library with ConsoleHelper + tests
Thanks a ton for this, @webimpress! |
Rewrite library with ConsoleHelper + tests.
Rebase needed after releasing v1.1.1 with fixes #5, #6, #7.
These changes provide new script:
Commands:
help
- display help/usage messageenable
- enable composer autoloading for the moduledisable
- disbale composer autoloading for the moduleOptions are the same as previously:
--help|-h
- display help/usage message--composer|-c
- composer binary path; defaults composer--type|-t
- module autoloading type to use--modules-path|-p
- path to directory with modulesAlso added full test coverage. Please enable coveralls for the repository.
Note
This version will be used in zendframework/zend-expressive-tooling#12 as it provides functionality also to disable module's composer autoloading.