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

Display sorted command list #11203

Merged
merged 1 commit into from Sep 18, 2017

Conversation

Projects
None yet
4 participants
@rchavik
Member

rchavik commented Sep 16, 2017

Better UX

@ADmad

This comment has been minimized.

Show comment
Hide comment
@ADmad
Member

ADmad commented Sep 16, 2017

@rchavik

This comment has been minimized.

Show comment
Hide comment
@rchavik

rchavik Sep 16, 2017

Member

What is it talking about?

Member

rchavik commented Sep 16, 2017

What is it talking about?

@ADmad

This comment has been minimized.

Show comment
Hide comment
@ADmad

ADmad Sep 16, 2017

Member

Problem is due to line 76 $this->commands = $this->getCommands();. getCommands() returns an array and you can use getIterator() on an array. So the code needs to account for the fact that $this->commands can be an array or \Cake\Console\CommandCollection instance. Also the typehint of $commands property needs to be updated.

Member

ADmad commented Sep 16, 2017

Problem is due to line 76 $this->commands = $this->getCommands();. getCommands() returns an array and you can use getIterator() on an array. So the code needs to account for the fact that $this->commands can be an array or \Cake\Console\CommandCollection instance. Also the typehint of $commands property needs to be updated.

@ADmad

This comment has been minimized.

Show comment
Hide comment
@ADmad

ADmad Sep 16, 2017

Member

Also there is currently no test coverage for line 76 or the getCommands() method.

Member

ADmad commented Sep 16, 2017

Also there is currently no test coverage for line 76 or the getCommands() method.

@markstory markstory added this to the 3.5.3 milestone Sep 17, 2017

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Sep 17, 2017

Codecov Report

Merging #11203 into master will increase coverage by <.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11203      +/-   ##
============================================
+ Coverage     93.14%   93.15%   +<.01%     
  Complexity    12981    12981              
============================================
  Files           437      437              
  Lines         33623    33626       +3     
============================================
+ Hits          31318    31324       +6     
+ Misses         2305     2302       -3
Impacted Files Coverage Δ Complexity Δ
src/Shell/HelpShell.php 80.32% <83.33%> (+1.01%) 17 <0> (ø) ⬇️
src/Cache/CacheRegistry.php 100% <0%> (+4%) 11% <0%> (ø) ⬇️
src/Cache/CacheEngine.php 93.61% <0%> (+4.25%) 19% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b85ef39...a931060. Read the comment docs.

codecov-io commented Sep 17, 2017

Codecov Report

Merging #11203 into master will increase coverage by <.01%.
The diff coverage is 83.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master   #11203      +/-   ##
============================================
+ Coverage     93.14%   93.15%   +<.01%     
  Complexity    12981    12981              
============================================
  Files           437      437              
  Lines         33623    33626       +3     
============================================
+ Hits          31318    31324       +6     
+ Misses         2305     2302       -3
Impacted Files Coverage Δ Complexity Δ
src/Shell/HelpShell.php 80.32% <83.33%> (+1.01%) 17 <0> (ø) ⬇️
src/Cache/CacheRegistry.php 100% <0%> (+4%) 11% <0%> (ø) ⬇️
src/Cache/CacheEngine.php 93.61% <0%> (+4.25%) 19% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b85ef39...a931060. Read the comment docs.

@rchavik

This comment has been minimized.

Show comment
Hide comment
@rchavik

rchavik Sep 17, 2017

Member

how about now, phpstan seems happy.

Member

rchavik commented Sep 17, 2017

how about now, phpstan seems happy.

@markstory markstory merged commit a922ebd into master Sep 18, 2017

4 of 6 checks passed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
codecov/patch 83.33% of diff hit (target 93.14%)
Details
codecov/project 93.15% (+<.01%) compared to b85ef39
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
stickler-ci No lint errors found.

@markstory markstory deleted the 3.5-sorted-commands branch Sep 18, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment