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.
The variable 'smtp_library' seems to be for drupal 6 only. It's confusing for developers that it still exists in the drupal-7 version of the module. I guess it woudl be completely safe just to remove it (but of course keep the code in the uninstall-hook).
Comment | File | Size | Author |
---|---|---|---|
#14 | Screen shot 2016-01-30 at 10.07.06 PM.png | 30.05 KB | Anonymous (not verified) |
#6 | smtp-n1889278-6.patch | 2.09 KB | DamienMcKenna |
|
Comments
Comment #1
gnucifer CreditAttribution: gnucifer commented'smtp_on' can also be removed I guess?
Comment #2
Simon Georges CreditAttribution: Simon Georges commentedOk, let's remove
smtp_library
. Changing category as it is not really a bug.This patch is part of the #1day1patch initiative.
Comment #3
wundo CreditAttribution: wundo commentedSimon,
looks like you're touching more code than just removing the smtp_library variable, could you explain what you're doing?
Comment #4
Simon Georges CreditAttribution: Simon Georges commentedWell, everywhere there's mention of smtp_library (outside of .install), I'm trying to adapt the code. Since mimemail does not define it either, there's a part of the code that could (should?) go too. I just let the status test using the smtp_on variable, but I think the rest is useless. It clearly needs a thorough review.
Comment #5
Simon Georges CreditAttribution: Simon Georges commentedAs #1 is mentionning
smtp_on
, I'm cross-referencing #1663008: Variable "smtp_on" doesn't actually disable anything....Comment #6
DamienMcKennaI've updated the patch to add an update script that deletes the variable, and then there's no need for the lines in smtp_uninstall.
Comment #8
DamienMcKennaComment #9
DamienMcKennaThe test failed because of a bug in drupalci: #2645590: Ensure that simpletest job doesn't "fail" testing if no tests are present
Comment #10
DamienMcKennaThe tests failed because of a bug in DrupalCI: #2645590: Ensure that simpletest job doesn't "fail" testing if no tests are present
Comment #12
DamienMcKennaComment #13
DamienMcKennaThis would be safe to include in the next release.
Comment #14
Anonymous (not verified) CreditAttribution: Anonymous as a volunteer and commentedThis installed perfect for me. That said, it was in a clean install with no other patches applied. I wanted to check in ideal conditions. Let me know if I should test this patch with other patches applied as well.
Comment #15
gadaniels72 CreditAttribution: gadaniels72 as a volunteer commentedThis worked for me as well. Tested both on a clean install and with some of the other needs review patches installed.
Comment #17
wundo CreditAttribution: wundo at Chuva Inc. for Chuva Inc. commented