Hey there,

Awesome module, thanks for contributing it! In looking into it a bit, I saw that the module still requires the 1.x version of spatie/schema-org. Would it be possible to allow v2 to be installed, as well?

Thanks.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler created an issue. See original summary.

Sam152’s picture

Thanks! I would support this, but don't have the time to write a patch at the moment.

tstoeckler’s picture

Status: Active » Needs review
FileSize
273 bytes

No problem. Here it is.

So regarding BC, the 2.0.0 release states:

This release probably doesn't contain any breaking changes for most people. It breaks all type checks (e.g. instanceof LocalBusiness), but doesn't change the way types are generated

So I think this should be acceptable.

Status: Needs review » Needs work

The last submitted patch, 3: 3041720-3.patch, failed testing. View results

rkbrasse’s picture

Anything I can do to help move this along. I need to use one of the more recently updated Schema's in 2.2.

zaporylie’s picture

Tests are failing because
{"@context":"http:\/\/schema.org","@type":"Thing","name":"Foo"}
doesn't match (mind the protocol)
{"@context":"https:\/\/schema.org","@type":"Thing","name":"Foo"}

This is the only BC break covered by tests. I agree with @tstoeckler that it should be fine to update version constraint, especially that this module is still in alpha.

@rkbrasse If you want to help to move this along you may wanna provide a patch that fixes tests.

Sam152’s picture

Thanks for looking into this. I'll commit a patch that bumps the library and fixes the tests.

Probably just worth a note in the new release notes on the nature of supporting two major versions of a library.

zaporylie’s picture

Status: Needs work » Needs review
FileSize
2.86 KB

Here's fix for tests combined with update to version constraint in composer.json (#3).

  • Sam152 committed ffc4a17 on 8.x-1.x authored by zaporylie
    Issue #3041720 by tstoeckler, zaporylie: Support spatie/schema-org v2
    
Sam152’s picture

Status: Needs review » Fixed

Fixed, thanks for the patch. Cutting a new release now!

Sam152’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.