Skip to content

Add run command, refactor tests #72

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

Merged
merged 1 commit into from
Jan 26, 2016
Merged

Add run command, refactor tests #72

merged 1 commit into from
Jan 26, 2016

Conversation

mikeymike
Copy link
Member

Hoping this is all right, I was behind master so rebased but I had to manually resolve some conflicts. I think it's okay but could be wrong 😂

Also got to double check Travis tests

@codecov-io
Copy link

Current coverage is 89.73%

Merging #72 into master will decrease coverage by -1.57% as of b8db6b2

@@            master     #72   diff @@
======================================
  Files           68      69     +1
  Stmts         1576    1656    +80
  Branches         0       0       
  Methods        268     274     +6
======================================
+ Hit           1439    1486    +47
  Partial          0       0       
- Missed         137     170    +33

Review entire Coverage Diff as of b8db6b2

Powered by Codecov. Updated on successful CI builds.

@mikeymike
Copy link
Member Author

darn it also dropped coverage 👎

@mikeymike
Copy link
Member Author

hmmm just Travis :trollface:

@mikeymike
Copy link
Member Author

@AydinHassan I can't re-create the test failing locally on any PHP version. Can you pull down and run them. On Travis 7.0 failed then after re-run 7.0 passed and 5.6 failed. Pretty sure it was on the same test too.

/**
* @param Request $request
*/
public function writeRequest(Request $request);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should type hint the PSR RequestInterface

@mikeymike
Copy link
Member Author

@AydinHassan I'll rebase before merge if all is good 👍

@mikeymike
Copy link
Member Author

my bad didn't re-run the test 🐰

@AydinHassan
Copy link
Member

@mikeymike This ready? Can we squash it?

@mikeymike
Copy link
Member Author

Yeah can do, I did just realise I hadn't written a test for the RunCommand itself 😑 ... I can do this after the merge though, got nothing better to be doing 😆

What do you think ?

@AydinHassan
Copy link
Member

Yeah, fine by me 😄 That's probably why the coverage went down!

@mikeymike mikeymike force-pushed the feature/run-command branch from 8be7307 to b8db6b2 Compare January 26, 2016 22:30
AydinHassan added a commit that referenced this pull request Jan 26, 2016
@AydinHassan AydinHassan merged commit dc93f97 into master Jan 26, 2016
@AydinHassan AydinHassan deleted the feature/run-command branch January 26, 2016 22:33
@AydinHassan AydinHassan mentioned this pull request Jan 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants