Not 100% sure of the EntityResource one, but definitely the DBlogResource plugin should be moved.

CommentFileSizeAuthor
#5 1875118-5.patch2.76 KBdamiankloip
#1 rest-1875118-1.patch2.95 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new rest-1875118-1.patch2.95 KB

Note that the files are moving via git.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Needs work

Yes, this makes sense.

+++ b/core/modules/dblog/lib/Drupal/dblog/Plugin/rest/resource/DBLogResource.php
@@ -19,23 +18,13 @@
+ *   module = "dblog"

Why is that needed? You could easily determine the module by the namespace of the plugin class. And if we do not have an explicit use case for the module information I would say let's omit it.

Or is this a plugin standard now?

+++ b/core/modules/entity/lib/Drupal/entity/Plugin/rest/resource/EntityResource.php
@@ -23,6 +23,7 @@
+ *   module = "entity",

Same here.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

Also: what about the test cases? Should they remain in REST module or should they be moved as well?

I really don't care much, but it might be confusing to have a Drupal core module that does not have a single simpletest :-D

liuggio’s picture

liuggio commented
Issue summary: View changes
Related issues: +#2063603: Rest uses RouteCollection without a use statement

The other issue has been closed as duplicated as this one.

damiankloip’s picture

damiankloip commented
Status: Needs work » Needs review
StatusFileSize
new 1875118-5.patch2.76 KB

Just spoke to klausi about this. Let's just move the db log resource plugin and the corresponding test.

We should then open a follow up to discuss where we should add documentation about finding examples for these plugins.

Crell’s picture

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

Seems sane enough.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Looks sensible.

Committed and pushed to 8.x. Thanks!

  • webchick committed 8ca26dc on 8.x 
    Issue #1875118 by damiankloip, tim.plunkett: Move resource plugins to...

Status: Fixed » Closed (fixed)

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