Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
documentation
Priority:
Minor
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
22 Apr 2015 at 21:55 UTC
Updated:
17 May 2015 at 04:14 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
jhodgdonYeah it was made into a plugin system in Drupal 8. Should refer to:
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Queue!QueueWorker...
Comment #2
pixel5 commentedI'm new to OOP naming conventions, but I think I got this. Hopefully it's good.
Comment #3
cilefen commented@pixel5 Good work. Comment lines must wrap before 80 columns so this line is too long. Don't fix that until we find out if it must be fully-qualified. I think it does.
@jhodgdon I assume class names must be fully-qualified to work properly on the API pages. Is that true?
Comment #4
disasm commentedGreat first patch! The one thing I would mention is in comments we fully qualify the namespace of the class we're linking to, and also make sure the comment doesn't exceed 80 chars.
This should be: \Drupal\Core\Queue\QueueWorkerInterface::processItems
Thanks for getting this issue rolling!
Comment #5
pixel5 commentedGot it, will use FQNS in the future. Comment lines are now under 80 columns. This patch should do it then.
Comment #6
cilefen commented@pixel Very good. There is just one issue I can see - there are trailing spaces at the end of the lines you edited.
Also, could you please use dreditor to insert a Beta Evaluation in the Issue Summary? Documentation changes are one of the allowed categories so you will uncomment that line. You will see what I mean when you paste in the evaluation. Once that is done, please remove the "Needs beta evaluation" tag.
Comment #7
pixel5 commentedAlright, fixed those issues. Got codesniffer installed, it doesn't show that there are any problems with the code. Third time's a charm?
Comment #8
cilefen commentedOk, this is almost done. From having looked around at core.api.php and the standards page, I think that the method name must end with parenthesis to work properly on the API site so it would be
\Drupal\Core\Queue\QueueWorkerInterface::processItems().Comment #9
pixel5 commentedLOL, figures! Okay. Got it.
Comment #10
disasm commentedOne minor alteration. After that, I'm ready to mark this RTBC.
since you got rid of "may" prefixing throw, throw on line 15 should be throws.
Comment #11
pixel5 commentedDone.
Comment #12
cilefen commentedIt should be
\Drupal\Core\Queue\QueueWorkerInterface::processItem().Comment #13
pixel5 commentedComment #14
cilefen commentedThis looks good to me.
Comment #15
webchickNice catch!
Committed and pushed to 8.0.x. Thanks!