Fix use of invariant() calls when the string is not constant #77
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks! If you are contributing on behalf of someone else (eg your employer): the individual CLA is not sufficient - use https://developers.facebook.com/opensource/cla?type=company instead. Contact [email protected] if you have any questions. |
| invariant(is_array($v), $k.' is not an array'); | ||
| invariant(is_array($v), | ||
| '%s', | ||
| $k.' is not an array'); |
There was a problem hiding this comment.
'%s is not an array', $kwould be nicer- indent, don't align:
invariant(
is_array($v),
'%s',
....| @@ -83,7 +83,7 @@ private static function RunWithOptionsAndEngine( | |||
| $php_engine->start(); | |||
| Process::sleepSeconds($options->delayPhpStartup); | |||
| invariant( | |||
| $php_engine->isRunning(), | |||
| $php_engine->isRunning(), '%s', | |||
| @@ -129,7 +129,7 @@ private static function RunWithOptionsAndEngine( | |||
|
|
|||
| invariant(!$siege->isRunning(), 'Siege is still running :/'); | |||
| invariant( | |||
| $php_engine->isRunning(), | |||
| $php_engine->isRunning(),'%s', | |||
There was a problem hiding this comment.
newline - and if there wasn't one, you'd want a space
|
#78 addressed this |
Some of the invariant tests are using strings composed of a concatenation and not a simple string. Hack type check will barf on those. Fixed to set the format string to "%s" and the string a parameter to conform to hack type checking standard.