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.
After I manually run lightweight cron on admin/config/content/scheduler/cron
I have the message 'Lightweight cron run completed - see log for details.'
Since I am using syslog, the link to dblog doesn't exist.
We need to add a check and only show the dblog message if the module is enabled.
Comment | File | Size | Author |
---|---|---|---|
#29 | 2611686-29-interdiff.txt | 1 KB | pfrenssen |
#27 | only_display_dblog-2611686-27.patch | 3.04 KB | joekers |
| |||
#2 | dblog-2611686-1.patch | 2.11 KB | marcelovani |
Screen Shot 2015-11-09 at 12.33.13.png | 39.72 KB | marcelovani |
Comments
Comment #2
marcelovaniD7 patch
Comment #3
marcelovaniD8 Patch
Comment #4
joekers+ if (\Drupal::moduleHandler()->moduleExists('block')) {
I think this should be checking for the dblog module instead of block? :)
Other than that it looks good to me.
Comment #6
marcelovaniha, good spot, copy > paste > forgot to change it LOL
Comment #7
marcelovaniComment #9
marcelovaniComment #12
marcelovaniComment #13
marcelovaniComment #15
marcelovaniLooks like D8 tests are not passing at all.
Comment #17
marcelovaniComment #18
joshi.rohit100Should be injected
Comment #19
marcelovanierr, did you forget to attach your path?
Comment #20
joshi.rohit100Nope! I just reviewed your patch at #7.
Comment #21
marcelovaniSince you know a better way of doing it, it would be nice if you could collaborate with a patch
Comment #22
joshi.rohit100I didn't provide the patch as it was assigned to you. Now assigning myself :)
Comment #23
marcelovanicool
Comment #24
joekersSomething like this @joshi.rohit100?
Sorry to jump in there but I've never used DI in Drupal 8 so I thought it would be a good opportunity to try :)
Comment #26
joshi.rohit100should be "Creates a SchedulerCronForm instance."
missing parameter name
Missing the use statement.
Also it would be nice if we just change module_handler to moduleHandler (only property not in constructor) as core uses this name.
Patch is there, so unassigning!
Comment #27
joekersThanks for the feedback @joshi.rohit100.
Comment #28
joshi.rohit100Comment #29
pfrenssenThis is much better, nice improvement! Thanks a lot guys!
I removed an unused use statement and fixed some minor coding standards before committing.
Comment #31
jonathan1055 CreditAttribution: jonathan1055 commentedThanks, good work.
As this issue was originally raised at 7.x I think it will be worth making the fix there too. Seems like the original patch at #1 is correct? Are you ok if I commit it?
Comment #32
pfrenssenYes, absolutely! I didn't intend to bypass this for D7. Patch looks good!
Comment #34
jonathan1055 CreditAttribution: jonathan1055 commentedDone. Thanks marcelovani for the original 7.x patch
Comment #38
jonathan1055 CreditAttribution: jonathan1055 commentedIgnore the failed patch above. It was queued but not run until the 8.x committed codebase passed all tests. That happened with my commit a few minutes ago, and then the untested patches have suddenly come to life and been run. This issue is already fixed.