simialbi / yii2-schema-org

Schema.org yii2 representation and helpers for json-ld generation
MIT License
17 stars 4 forks source link

Rewrite the generation process #5

Closed machour closed 5 years ago

machour commented 5 years ago

Hey @simialbi,

Here's my take on the generation in this PR.

The new command needs to be ran with:

For example:

./yii schema/schema-org  --schemas=Car,AutoDealer,Brand,Offer --namespace='common\schemas' --folder common/schemas

You can also use a particular version of schemas.org by passing the version number in the call:

./yii schema/schema-org 3.1 --schemas=Car,AutoDealer,Brand,Offer --namespace='common\schemas' --folder common/schemas

The generator make uses of traits for all needed schemas (the one requested by the user and all their parents). I had to use traits because of some cases of multiple inheritance like this one.

The generator will only generate public classes for the schemas passed in the options (they just require their own traits).

Could you give it a try and tell me what you think ?

Still need to:

machour commented 5 years ago

Hey,

I think I'm done with this PR, you can check out the new README.md file here: https://github.com/machour/yii2-schema-org/tree/new-generator

I've added tests that generates all files for schema release 3.4 and validates all generated files by including them and checking for class/trait existence.

You can run tests using: composer install && phpunit -c phpunit.xml.dist

That allowed me to find another problem with Class and Float that are reserved keywords. I prefixed them with sc.

I am not really used to write tests, so you may want to have a look at what I'm doing there.

Tests should be hooked up by Travis CI.

Have a great week end!

simialbi commented 5 years ago

Hey @machour You did a great job with your suggestion, but I think the usage of traits has a big disadvantage. E.g. if you just need Offer, your generator generates at least 4 Files:

  1. ThingTrait
  2. OfferTrait
  3. Thing
  4. Offer

Based on your ideas and work I refactored the generator this way: I traverse all the dependency tree to find all inherited properties of each class and generate complete classes. All of these classes inherit from Model. This method generates less but bigger files and they are more/completely independent... I pushed to simialbi/yii2-schema-org...master. For you ok?

machour commented 5 years ago

I was also coming to the same conclusion regarding traits, we're in sync :)

I see you're still adapting tests, ping when you're done with the changes to that I can pull master and see how well all of this work now.

I'd strongly advise bumping the next release to 2.0 as we're clearly breaking compatibility for old installations.

simialbi commented 5 years ago

@machour: Ok have a look. I hope it suits your needs. I'm going to do some tests in a real yii console environment. If they pass (and you agree), I'll publish a release.

I'd strongly advise bumping the next release to 2.0 as we're clearly breaking compatibility for old installations.

It should be 100% backwards compatible with default options (see updated README)

machour commented 5 years ago

Hey @simialbi, sorry got stuck in meetings :)

I've tested the latest beta you published, it looks great!

A minor thing: properties comments all contain HTML tags:

screen shot 2018-11-27 at 4 32 58 pm

If this is intended, maybe you could strip those <br/> as they won't add anything for phpDoc generation.

All classes but Model.php are gone, so this could really mess with old installations. I'd definitely go with 2.0, but as a new user, I'm fine either ways.

Thank you a lot for being so quick to respond and update the library!