Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
base system
Priority:
Major
Category:
Task
Assigned:
Issue tags:
Reporter:
Created:
9 Jul 2013 at 19:41 UTC
Updated:
7 Oct 2014 at 09:50 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
tim.plunkettComment #3
tim.plunkett#1: queue-2038275-1.patch queued for re-testing.
Comment #4
tim.plunkettAs part of #2044203: [meta] Convert info hooks to plugins
Comment #5
dawehnerAs written one day in IRC the name is quite odd. If nothing helps choose QueuePlugin
Comment #6
tim.plunkettYes, thanks for reminding me.
Comment #7
tim.plunkettLet's just go with that.
Comment #8
dawehnerI guess we should also rename the plugin manager service then.
No caching?
Comment #9
tim.plunkettComment #10
dawehnerI guess removing the tag was not intended.
Comment #11
tim.plunkettNo, it was incorrectly tagged. No one actually approved this, I was mistaken.
Comment #12
yesct commentedComment #13
sam hermans commentedI'll check this out
Comment #14
sam hermans commentedRe rolled against latest HEAD
Comment #15
dawehnerThis requires "\Drupal" now.
Let's ensure to not forget this todo before commit
Comment #18
mr.baileysThe parent constructor (DefaultPluginManager) only has three parameters, the array() here is wrong.
Drupal::service('plugin.manager.queue.item');to use\Drupal(reported in #15).Comment #19
mr.baileysTo the testbot!
Comment #21
mr.baileysShould be 'Plugin/QueuePlugin' as $subdir.
Comment #24
tim.plunkettRerolling.
Comment #25
tim.plunkettUpdated for core changes, added an interface for the plugin manager, other fixes. Big interdiff.
Comment #26
neclimdulAfter talking out loud in IRC I think my concern with this is currently the terminology. The plugin stuff we're doing isn't really connected to the QueueAPI, we're replacing the hook callback runner weirdness(which was sort of its own thing) with a runners based on plugins.
I think I would prefer this if the QueueManager was maybe QueueRunnerManager or QueueProcessorManager and if the surrounding implementation followed that terminology. That'll seperate it from the Queue and QueueItem concepts that already exist. I don't know what is the right term but I think either of those would be acceptable until I hear something better.
Comment #27
tim.plunkettAgreed, I really disliked the previous naming (QueuePlugin was such a cop-out)
EDIT: We discussed in IRC and landed on QueueWorker.
Comment #28
jibranI think @neclimdul is happy know. I have seen him ++ @tim.plunkett in IRC. I reviewed the patch and didn't find anything objectionable so this is good to go.
Why we need this class?
Oh, I got it. This is the reason. :)
Comment #29
damiankloip commentedWe should really not do this, but this is not the fault of this issue.
Any reason we are removing those messages? Just wondering.
Comment #30
alexpottWe need to bring this in line with the plugin api documentation efforts see
\Drupal\Core\Annotation\Actionfor an example.Comment #31
alexpottNeeds docs update for the aforementioned plugin api docs standards.
Can remove
use Drupal\aggregator\FeedInterface;from aggregator.module nowMissing docs
Where has this documentation gone? I would expect it to be on the QueueWorkerInterface.
Also the issue summary could be better and maybe we need a change record (8.x to 8.x I think).
Comment #32
jhodgdonalexpott asked me to help with the docs here.
What we need is docs like what was added for each child issue of #2269389: [meta] Make sure plugin developer info is discoverable. The child issues all have detailed instructions; one example: #2290281: Tip plugin classes need more docs links. It should take about 5 minutes to do...
Comment #33
tim.plunkettWill work on a change notice now.
Comment #34
jhodgdonThe docs additions look good, at least as far as formatting and completeness. Not being involved in this issue, I haven't reviewed them for accuracy. Thanks!
Comment #35
tim.plunkettPosted https://www.drupal.org/node/2341649, we can unpublish https://www.drupal.org/node/1537256 when this goes in.
Comment #36
catchCouple of docs fixes. Looks good otherwise.
Not a hook any more.
method? worker?
Comment #37
tim.plunkettThanks @catch, I fixed up the docs on both of those to be consistent with the changes in #33.
Comment #38
alexpottCommitted ac0cee8 and pushed to 8.0.x. Thanks!