Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of #1971384: [META] Convert page callbacks to controllers
For instructions on how to convert a page callback into a controller, see the WSCCI Conversion Guide.
Comment | File | Size | Author |
---|---|---|---|
#18 | 1987828-route-system-cron-run-18.patch | 3.3 KB | mtift |
#18 | interdiff.txt | 1.19 KB | mtift |
#15 | 1987828-route-system-cron-run-15.patch | 3.18 KB | mtift |
#15 | interdiff.txt | 1.8 KB | mtift |
#12 | 1987828-route-system-cron-run-12.patch | 2.88 KB | mtift |
Comments
Comment #1
vijaycs85Initial patch...
Comment #2
vijaycs85Adding hook_menu change...
Comment #3
ParisLiakos CreditAttribution: ParisLiakos commentedMisses @return docs (see below)
i think calling it runManually sounds better
Should return a RedirectResponse.
sth like:
dont forget the use statement;)
Comment #4
mtiftI think this patch addresses the items in #3
Comment #6
ParisLiakos CreditAttribution: ParisLiakos commentedi think you changed the existing method:P
you should apply the patch from #2 first
Comment #7
mtiftRe-rolled the patch and consolidated all of the changes
Comment #8
ParisLiakos CreditAttribution: ParisLiakos commentedstill the changes are on the existing method. run() should stay as is, runManual() is the method i was reffering above
Comment #9
mtiftAhh. That makes more sense.
Comment #11
ParisLiakos CreditAttribution: ParisLiakos commentedstill those changes are irrelevant.
instead this drupal_goto should be replaced with the RedirectResponse above
and after you move it, this should be updated to reflect the @return value
Comment #12
mtiftUpdated patch attached
Comment #13
ParisLiakos CreditAttribution: ParisLiakos commentedlooks a lot better now, thanks!
really minor: while in documentation we should always use the full namespace of the class (including preceding \)
lets update the method name here to reflect reality:)
Comment #14
dawehnerIf you have a MENU_CALLBACK you can directly remove the entry in hook_menu.
Comment #15
mtiftHere's an updated patch
Comment #16
dawehnerJust to note: On the longrun there should be something like a cronRunner service, which has everything injected.
it should be url('admin/reports/status', array('absolute' => TRUE));
Just remove all of it
Comment #17
ParisLiakos CreditAttribution: ParisLiakos commentedAgreed, there is already a @todo in the method above:)
Comment #18
mtiftRevised patch attached
Comment #20
ParisLiakos CreditAttribution: ParisLiakos commented#18: 1987828-route-system-cron-run-18.patch queued for re-testing.
Comment #22
ParisLiakos CreditAttribution: ParisLiakos commented#18: 1987828-route-system-cron-run-18.patch queued for re-testing.
Comment #23
ParisLiakos CreditAttribution: ParisLiakos commentedthats ready, thanks
Comment #24
catchCommitted/pushed to 8.x, thanks!