Minor fixups to CI and make it run PHPCS

wmde / Number

Numerical value objects, parsers and formatters

Other

18 stars 6 forks source link

Minor fixups to CI and make it run PHPCS #98

Closed thiemowmde closed 7 years ago

JeroenDeDauw commented 7 years ago

I took the liberty of amending and have travis call composer test instead of ci for now, since the phpcs checks apparently do not pass yet.

The sudo setting is not required. It tells travis that sudo is not needed during execution which allows them to use cheaper containers or something like that.

JeroenDeDauw commented 7 years ago

When I run the test locally I get a big pile of errors: https://pastebin.com/ew8SCZiX

php --version PHP 7.0.15-1ubuntu4 (cli) (built: Feb 28 2017 21:33:59) ( NTS ) Copyright (c) 1997-2017 The PHP Group Zend Engine v3.0.0, Copyright (c) 1998-2017 Zend Technologies with Zend OPcache v7.0.15-1ubuntu4, Copyright (c) 1999-2017, by Zend Technologies

Not related to this PR tho, also have that on master.

thiemowmde commented 7 years ago

I removed the sudo thing.
I changed Travis back to call composer ci, but changed composer ci to not run PHPCS for now. Fixed in #99.
The test failures are because your local system does not have the bcmath PHP extension installed. Fixed in #100.

I believe we should merge this as it is now.

manicki commented 7 years ago

This looks good. Commit message does say something else the change actually does in the end, though. Do we care? Or should I just shut up and merge?

manicki commented 7 years ago

Commit message says: Make CI use PHPUnit installed via Composer This change does not really do it, as it already happens on master.

This inconsistency is what I meant in my commet?

thiemowmde commented 7 years ago

I edited the title of this pull request an hour ago. I never look at the commit headlines here on GitHub. Now I found what you meant and changed it.

manicki commented 7 years ago

Yeah, I sometimes fail to look at PR titles :) git > github