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.

Files: 
CommentFileSizeAuthor
#13 1935548-13-remove-jsonld.patch45.42 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 55,489 pass(es).
[ View ]
#9 drupal-1935548-9.patch44.94 KBc4rl
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#4 1935548-04-remove-jsonld.patch48.22 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 53,927 pass(es).
[ View ]
#2 1935548-02-remove-jsonld.patch47.54 KBlinclark
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]
#1 1935548-01-remove-jsonld.patch108.03 KBlinclark
PASSED: [[SimpleTest]]: [MySQL] 52,593 pass(es).
[ View ]
#1 1935548-01-remove-jsonld__for-review.txt63.58 KBlinclark

Comments

linclark’s picture

Status:Active» Needs review
StatusFileSize
new63.58 KB
new108.03 KB
PASSED: [[SimpleTest]]: [MySQL] 52,593 pass(es).
[ View ]

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

linclark’s picture

StatusFileSize
new47.54 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

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.

linclark’s picture

Status:Needs work» Needs review
StatusFileSize
new48.22 KB
PASSED: [[SimpleTest]]: [MySQL] 53,927 pass(es).
[ View ]

Needed to remove a line in RdfBundle.

Crell’s picture

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?

linclark’s picture

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

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

Status:Reviewed & tested by the community» Needs work
c4rl’s picture

Status:Needs work» Needs review
StatusFileSize
new44.94 KB
FAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.
[ View ]

Re-rolled

Status:Needs review» Needs work

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

c4rl’s picture

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.

linclark’s picture

Status:Needs work» Needs review
StatusFileSize
new45.42 KB
PASSED: [[SimpleTest]]: [MySQL] 55,489 pass(es).
[ View ]

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

Status:Needs review» Reviewed & tested by the community

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

alexpott’s picture

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.