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

CommentFileSizeAuthor
#14 Screen shot 2016-01-30 at 10.07.06 PM.png30.05 KBAnonymous (not verified)
#6 smtp-n1889278-6.patch2.09 KBdamienmckenna
#2 smtp-1889278-2-remove_smtp_library_variable.patch1.97 KBsimon georges

Comments

gnucifer’s picture

gnucifer commented

'smtp_on' can also be removed I guess?

simon georges’s picture

simon georges commented
Category: bug » task
Status: Active » Needs review
StatusFileSize
new smtp-1889278-2-remove_smtp_library_variable.patch1.97 KB

Ok, let's remove smtp_library. Changing category as it is not really a bug.

This patch is part of the #1day1patch initiative.

wundo’s picture

wundo commented
Status: Needs review » Needs work

Simon,
looks like you're touching more code than just removing the smtp_library variable, could you explain what you're doing?

simon georges’s picture

simon georges commented

Well, 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.

simon georges’s picture

simon georges commented

As #1 is mentionning smtp_on, I'm cross-referencing #1663008: Variable "smtp_on" doesn't actually disable anything....

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Issue summary: View changes
Status: Needs work » Needs review
StatusFileSize
new smtp-n1889278-6.patch2.09 KB

I'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.

Status: Needs review » Needs work

The last submitted patch, 6: smtp-n1889278-6.patch, failed testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Title: Remove smtp_library from 7.x-version » Remove smtp_library variable from 7.x-version
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review

The test failed because of a bug in drupalci: #2645590: Ensure that simpletest job doesn't "fail" testing if no tests are present

damienmckenna’s picture

damienmckenna
Location TN, USA
commented

The tests failed because of a bug in DrupalCI: #2645590: Ensure that simpletest job doesn't "fail" testing if no tests are present

Status: Needs review » Needs work

The last submitted patch, 6: smtp-n1889278-6.patch, failed testing.

damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Status: Needs work » Needs review
damienmckenna’s picture

damienmckenna
Location TN, USA
commented
Parent issue: » #2651788: Plan for SMTP 7.x-1.4 release

This would be safe to include in the next release.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new Screen shot 2016-01-30 at 10.07.06 PM.png30.05 KB

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

gadaniels72’s picture

gadaniels72 commented
Status: Needs review » Reviewed & tested by the community

This worked for me as well. Tested both on a clean install and with some of the other needs review patches installed.

  • wundo committed 46bee9d on 7.x-1.x authored by DamienMcKenna 
    Issue #1889278 by DamienMcKenna, Simon Georges: Remove smtp_library...
  • wundo committed b4d88e7 on 7.x-1.x authored by DamienMcKenna
    Issue #1889278 by DamienMcKenna, Simon Georges: Remove smtp_library...
wundo’s picture

wundo commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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