php-kafka / php-avro-schema-generator

PHP avro subschema merger and experimental PHP Class avro schema generator
BSD 3-Clause "New" or "Revised" License
5 stars 5 forks source link

#33 Schema generation extensions #47

Open Hubbitus opened 2 years ago

Hubbitus commented 2 years ago

Major refactoring with PhpClassPropertyType typing

Closes #33

Hubbitus commented 2 years ago

@nick-zh, please look I've fixed almost all tests, but there still 2 failed like:

PhpKafka\PhpAvroSchemaGenerator\Tests\Integration\Parser\ClassParserTest::testGetProperties
ParseError: syntax error, unexpected '|', expecting variable (T_VARIABLE)

/var/www/html/example/classes/SomeTestClass.php:83
/var/www/html/src/Parser/ClassParser.php:221
/var/www/html/src/Parser/ClassParser.php:158
/var/www/html/tests/Integration/Parser/ClassParserTest.php:56
phpvfscomposer:///var/www/html/vendor/phpunit/phpunit/phpunit:97

But that related to the \PhpKafka\PhpAvroSchemaGenerator\Example\SomeTestClass::$blaaaaaaaa declaration:

public int|string $blaaaaaaaa;

That use Union type declaration which is available since PHP 8.0.0. So they failed expectedly on PHP container based on version 7.4. Should I upgrade the container to version 8.0.0 or do I just need to disable 2-3 tests? FYI all tests passed on my machine locally with PHP 8.0.2.

nick-zh commented 2 years ago

I think it is ok for 3.x to drop php7.4 support, so you can go ahead and update the container to 8.0

Hubbitus commented 2 years ago

All tests passed now. There is coverage below 100% thought. Please say if you are generally willing to accept such PR and I could continue work to extend coverage and deal with some code-style warnings.

Or we wait your refactoring in terms of https://github.com/php-kafka/php-avro-schema-generator/discussions/49?

nick-zh commented 2 years ago

Yeah might be best to maybe hold of for the moment, since some changes / renaming might complicate things unnecessarily for you. Sry for the inconvenience. I hope i can tackle this next weekend. I already started, but still need to figure out some stuff