There was no objection to switching to HAL, and there is a patch to switch REST module to support HAL by default, so it should be safe to remove JSON-LD module now.

CommentFileSizeAuthor
#13 1935548-13-remove-jsonld.patch45.42 KBlinclark
#9 drupal-1935548-9.patch44.94 KBc4rl
#4 1935548-04-remove-jsonld.patch48.22 KBlinclark
#2 1935548-02-remove-jsonld.patch47.54 KBlinclark
#1 1935548-01-remove-jsonld.patch108.03 KBlinclark
#1 1935548-01-remove-jsonld__for-review.txt63.58 KBlinclark

Comments

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
StatusFileSize
new 1935548-01-remove-jsonld__for-review.txt63.58 KB
new 1935548-01-remove-jsonld.patch108.03 KB

This patch removes the module and the entry in MAINTAINERS.txt.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new 1935548-02-remove-jsonld.patch47.54 KB

This patch is a reroll, plus it removes the routes for the RDF site specific vocabulary... we don't have an RDF serialization to serve them in at this point.

Status: Needs review » Needs work

The last submitted patch, 1935548-02-remove-jsonld.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 1935548-04-remove-jsonld.patch48.22 KB

Needed to remove a line in RdfBundle.

Crell’s picture

Crell commented

Alas, poor JSON-LD. I knew it well, Horatio...

Normally we don't kick modules out of core unless there's a contrib module to inherit them. This is a new module so I don't know if that applies here, but should we make sure this code has a new home in contrib before we ax it here?

Anonymous’s picture

Anonymous (not verified) commented

I'd say not. I think that we don't want to use the drupal_jsonld custom media type anyway, and the code should be refactored significantly now that $context is part of the Serializer::serialize function signature. Its probably better to start from scratch.

damien tournoud’s picture

damien tournoud commented
Status: Needs review » Reviewed & tested by the community

To be honest, I am not sad to see this go, but I am looking forward to the next iteration in contrib!

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work

Needs a reroll since #1939660: Use YAML as the primary means for service registration landed

c4rl’s picture

c4rl commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1935548-9.patch44.94 KB

Re-rolled

Status: Needs review » Needs work

The last submitted patch, drupal-1935548-9.patch, failed testing.

c4rl’s picture

c4rl commented
Status: Needs work » Needs review

#9: drupal-1935548-9.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-1935548-9.patch, failed testing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new 1935548-13-remove-jsonld.patch45.42 KB

I'm guessing that the reroll failed because the RouteSubscriber I had removed from RDFBundle in my previous patch made it in to the .yml file. This patch removes that from the .yml file, so we'll see if it works.

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

And, we're green. All removals so not much to review.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed f406799 and pushed to 8.x. Thanks!

The patch didn't apply cleaning due to changes in json.info.yml also core/modules/jsonld/jsonld.services.yml was being left behind… fixed during commit as this is a straight removal.

I checked for change notices to update... there are none.

Status: Fixed » Closed (fixed)

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