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.
There are a lot of callings for deprecated watchdog function in SMTPMailSystem.php
Comment | File | Size | Author |
---|---|---|---|
#23 | deprecated_watchdog_function-2742773-22.patch | 3.81 KB | jorgik |
Comments
Comment #2
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #3
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #4
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #5
dpovshed CreditAttribution: dpovshed as a volunteer and at Drupal Ukraine Community commented@jorgik, there are 2 minor issues with your patch:
1) this line
\Drupal::logger('smtp')->error( 'There is no submitted from address.');
contains an extra space
2) In this line
drupal_set_message(t('The submitted from address @from is not valid.', array('@from' => $from)), 'error');
I believe no need to remove parenthesis around @from. Or at least we should change it to %from to emphasise core element of the message.
Comment #6
pazhyn CreditAttribution: pazhyn as a volunteer and at AnyforSoft, Drupal Ukraine Community commentedComment #7
pazhyn CreditAttribution: pazhyn as a volunteer and at AnyforSoft, Drupal Ukraine Community commentedComment #8
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #9
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #10
dpovshed CreditAttribution: dpovshed as a volunteer and at Drupal Ukraine Community commentedThank you, #9 looks nice!
Comment #11
estoyausenteIt looks nice but maybe we can inject the logger in place of the procedural way. I think that mail system are very relevant in all sites and it's useful make it configurable in the class constructor.
Review this link https://www.drupal.org/node/2595985 for checking it.
(But anyway, the patch apply correctly and is correct).
Comment #12
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedOK, I changed patch, added logger in constructor, needs review
Comment #13
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedComment #14
estoyausenteI think that you has mixed two different issues in the patch ;)
(The other part seems nice, please fix it and I will review after it)
Comment #15
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedSorry, fixed
Comment #18
pazhyn CreditAttribution: pazhyn as a volunteer and at AnyforSoft, Drupal Ukraine Community commentedProvided patch #15 is not working for me.
Errors in log:
...
Needs work.
Comment #19
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedTested on the last update from 8.x-1.x branch
Comment #20
estoyausenteThis isn't related with the current issue, are only indentation changes. Is good idea resolve one task in each issue. If you want to refactor the code to use the coding standard, please open another issue.
Comment #21
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedOk, fixed in new patch
Comment #22
estoyausenteI tried to send an email, and watchdog throw me this message:
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "smtp.logger.channel". en Drupal\Component\DependencyInjection\Container->get() (linea 157 de /home/rrqun/www/core/lib/Drupal/Component/DependencyInjection/Container.php).
Can you test it again, please?
Comment #23
jorgik CreditAttribution: jorgik at AnyforSoft, Drupal Ukraine Community commentedSorry, tested, with correct of name service http://joxi.net/a2X3vYGhyP5NYm.png
Comment #24
estoyausentePatch work perfect, messages save in watchdog correctly. I think that all is ok.
But please, in order to ease the review, make an interdiff in this kind of patchs with little changes between versions.
https://www.drupal.org/documentation/git/interdiff
Comment #25
wundo CreditAttribution: wundo at Chuva Inc. for Chuva Inc. commentedComment #27
wundo CreditAttribution: wundo at Chuva Inc. for Chuva Inc. commented