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.
Hi
While I brower the code of flippy, for the code in flippy_uninstall, I think we can improve it a little bit.
ref: http://drupalcode.org/project/boost.git/blob/refs/heads/7.x-1.x:/boost.i...
This looks like more efficient.
function boost_uninstall() {
// Clear variables
$name = 'boost_';
db_delete('variable')
->condition('name', db_like($name) . '%', 'LIKE')
->execute();
cache_clear_all('variables', 'cache_bootstrap');
}
I make a patch here, and hope it can help.
Thanks for your attention.
Any questions, feel free to let me know. thank you.
Comment | File | Size | Author |
---|---|---|---|
#5 | flippy_rewrite-flippy_uninstall-1981528-4.patch | 722 bytes | zterry95 |
#2 | flippy_rewrite-flippy_uninstall-1981528-1.patch | 670 bytes | zterry95 |
flippy_rewrite-flippy_uninstall.patch | 671 bytes | zterry95 | |
Comments
Comment #1
zterry95 CreditAttribution: zterry95 commentedComment #2
zterry95 CreditAttribution: zterry95 commentedsmall problem with last patch, pls use this one.
Already test it and works well.
Comment #3
rliGood point zterry95, will review and commit it soon.
thanks.
Comment #4
rliHi zterry95,
Just had one question about your patch. By comparing your patch and the original code, I see you did not include
unset($conf[$name]);
in your patch. But I thought it was necessary to unset this variable key as this variable will loaded in each page request.I am going to change the status to need work now, but please feel free to post any ideas here.
Thanks.
Comment #5
zterry95 CreditAttribution: zterry95 commentedHi rli
Thanks for your reminder.
You are right, we need to clean cache of bootstrap.
This has been emplemented in variable_del
http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/varia...
So I improve my patch.
Hope this time, it can be passed:)
Comment #6
zterry95 CreditAttribution: zterry95 commentedComment #7
rliHi zterry95,
I have been thinking about your patch.
If you have compared your patch and variable_del function, you would find out variable_del is unsetting the $conf['name'] which can not be done in this patch if you don't have the variable name.
To clean everything up in the uninstall, I think we should clean $conf['variable name'] too. Any ideas?
Comment #8
zterry95 CreditAttribution: zterry95 commentedconsidering your comments, I do 2 things just now.
1: tracking the code
2: test myself
The results it that it not necessary to unset $conf['variable name'].
the reason is that, after we implement
the variable is still on the module uninstall page. but if we redirect to other page or refresh page, the variable(flippy_*) will be unset from $conf automaticly.
and after we uninstall a module, then redirect to another page or refresh, it is a naturally operation. then the variable(flippy_*) doesn't stay in $conf any more.
So I think unset($conf[variable name]) is not necessary and my last patch will work pretty well.
Feel free if you have any questions on my answers. I will be glad to explain.
Comment #9
rliyeah, understood and agree with you.
just considered variable_del does include the unset for a reason. but cannot think of any examples now.
will commit to dev for now.
Comment #10
rliyeah, understood and agree with you.
just considered variable_del does include the unset for a reason. but cannot think of any examples now.
will commit to dev for now.
Comment #11
zterry95 CreditAttribution: zterry95 commentednice.
cheers!
Comment #12
rliComment #13
zterry95 CreditAttribution: zterry95 commentedglad to see this patch has been applied.
but a little dispointed that, this patch is not authored by me:(
You should add below parameter when you commit this.
git commit --author="zterry95 <zterry95@1952394.no-reply.drupal.org>"
Then this should be my commit.
Thanks for your attention.