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.

CommentFileSizeAuthor
#8 3041720-8.patch2.86 KBzaporylie
#3 3041720-3.patch273 byteststoeckler

Comments

tstoeckler created an issue. See original summary.

sam152’s picture

sam152 commented

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

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review
StatusFileSize
new 3041720-3.patch273 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

rkbrasse commented

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

zaporylie
he/him
Primary language Polish
commented

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

sam152 commented

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

zaporylie
he/him
Primary language Polish
commented
Status: Needs work » Needs review
StatusFileSize
new 3041720-8.patch2.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

sam152 commented
Status: Needs review » Fixed

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

sam152’s picture

sam152 commented

Release created: https://www.drupal.org/project/json_ld_schema/releases/8.x-1.0-rc1

Status: Fixed » Closed (fixed)

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