Fix PHP platform #430
Fix PHP platform #430
Conversation
| @@ -7,7 +7,7 @@ | |||
| "php": ">=7.3" | |||
| }, | |||
| "require-dev": { | |||
| "phpunit/phpunit": "^9.0" | |||
| "phpunit/phpunit": "^10" | |||
There was a problem hiding this comment.
Since the Class and the filename does not match only PHPUnit 10 is able to work
| * | ||
| * @return {Boolean} true is the specified email is valid, false otherwise | ||
| */ | ||
| namespace Fgribreau; |
There was a problem hiding this comment.
This is the main error.
In composer, we said Fgribreau namespace is in platform/php. But this was wrong. It was Fgribreau\PHP. So the class was not autoloadable.
Two way to fix it
- fix the namespace here, to remove PHP. I prefer this one.
PHPin a PHP namespace brings nothing - fix in composer json, to append
\\PHP
As you can see, I choose. This is a BC Break. BUT since it does not work anyway, this is a bugfix
BTW, the README was already OK
| @@ -39,9 +30,6 @@ public static function blacklist(): array | |||
| return array_keys(self::$blacklist); | |||
| } | |||
|
|
|||
| /** | |||
| * Check if an email is blacklisted or not. | |||
| */ | |||
There was a problem hiding this comment.
I removed useless PHPDoc
|
|
||
| require_once __DIR__ . '/../platform/php/MailChecker.php'; |
There was a problem hiding this comment.
We use the autoloader now, to ensure the composer setup is correct
There was a problem hiding this comment.
sadly npm run test:php does not run with these changes, I'm bringing it back:
require_once __DIR__ . '/../platform/php/MailChecker.php';
There was a problem hiding this comment.
I guess it's because you didn't run composer install before running npm run test:php
There was a problem hiding this comment.
indeed, we don't rely on composer currently (see package.json for the cross-language support)
No description provided.