Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the documentation is wrong.
Issue priority Minor because ...
Unfrozen changes Unfrozen because it only changes documentation.

The doxygen on SuspendQueueException says, "An implementation of callback_queue_worker() may throw this class of exception to indicate that processing of the whole queue should be skipped." But there's no such thing as callback_queue_worker() in Drupal 8, nor is there any other mention of it in the codebase.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Yeah 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...

pixel5’s picture

Status: Active » Needs review
FileSize
762 bytes

I'm new to OOP naming conventions, but I think I got this. Hopefully it's good.

cilefen’s picture

+++ b/core/lib/Drupal/Core/Queue/SuspendQueueException.php
@@ -10,7 +10,7 @@
- * An implementation of callback_queue_worker() may throw this class of
+ * An implementation of QueueWorkerInterface::processItem may throw this class of

@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?

disasm’s picture

Great 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.

+++ b/core/lib/Drupal/Core/Queue/SuspendQueueException.php
@@ -10,7 +10,7 @@
+ * An implementation of QueueWorkerInterface::processItem may throw this class of

This should be: \Drupal\Core\Queue\QueueWorkerInterface::processItems

Thanks for getting this issue rolling!

pixel5’s picture

Got it, will use FQNS in the future. Comment lines are now under 80 columns. This patch should do it then.

cilefen’s picture

Status: Needs review » Needs work
Issue tags: +Needs beta evaluation

@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.

pixel5’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs beta evaluation
FileSize
1.33 KB

Alright, fixed those issues. Got codesniffer installed, it doesn't show that there are any problems with the code. Third time's a charm?

cilefen’s picture

Issue summary: View changes
Status: Needs review » Needs work

Ok, 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().

pixel5’s picture

Status: Needs work » Needs review
FileSize
1.33 KB

LOL, figures! Okay. Got it.

disasm’s picture

Status: Needs review » Needs work

One minor alteration. After that, I'm ready to mark this RTBC.

+++ b/core/lib/Drupal/Core/Queue/SuspendQueueException.php
@@ -10,11 +10,11 @@
+ * throw this class of exception to indicate that processing of the whole queue

since you got rid of "may" prefixing throw, throw on line 15 should be throws.

pixel5’s picture

Status: Needs work » Needs review
FileSize
1.33 KB

Done.

cilefen’s picture

Status: Needs review » Needs work
+++ b/core/lib/Drupal/Core/Queue/SuspendQueueException.php
@@ -10,11 +10,11 @@
+ * An implementation of \Drupal\Core\Queue\QueueWorkerInterface::processItems()

It should be \Drupal\Core\Queue\QueueWorkerInterface::processItem().

pixel5’s picture

Status: Needs work » Needs review
FileSize
1.33 KB
cilefen’s picture

Status: Needs review » Reviewed & tested by the community

This looks good to me.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Nice catch!

Committed and pushed to 8.0.x. Thanks!

  • webchick committed fc0c278 on 8.0.x
    Issue #2476225 by pixel5, cilefen, disasm, jhodgdon, TravisCarden:...

The last submitted patch, 9: SuspendQueueException-doxygen-2476225-9.patch, failed testing.

The last submitted patch, 9: SuspendQueueException-doxygen-2476225-9.patch, failed testing.

Status: Fixed » Closed (fixed)

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