Skip to content

Check code for errors and warnings. #114

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

Closed
wants to merge 8 commits into from
Closed

Conversation

devnuhl
Copy link
Member

@devnuhl devnuhl commented May 24, 2016

Ref: #4

devnuhl added 4 commits May 22, 2016 13:18
warnings will still show in run output, as it is written differently. this is correct behavior.
do not force error to display, in fact, turn them off.
@codecov-io
Copy link

codecov-io commented May 24, 2016

Current coverage is 87.20%

Merging #114 into master will decrease coverage by 1.43%

@@             master       #114   diff @@
==========================================
  Files            69         69          
  Lines          1574       1648    +74   
  Methods         280        291    +11   
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           1395       1437    +42   
- Misses          179        211    +32   
  Partials          0          0          

Powered by Codecov. Last updated by 2075d9e...c2397ab

devnuhl added 4 commits May 28, 2016 03:14
remove some array keys that worked in some environments, but not all.
rewrite some test comparisons due to the changed formatting of some error messages.
ran composer update, so have new lock file.
);
}

public function getActual()
Copy link
Member

Choose a reason for hiding this comment

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

can we add return types on these two methods

@AydinHassan
Copy link
Member

Great work , as you can see there is a fair bit of feedback, sorry for that!

Also - we will have to introduce some unit tests here - the output comparison is a big part of the framework and I want to keep it solid.

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