Meta issue: #2205673: [meta] Remove all @deprecated functions marked "remove before 8.0"

Only the function exists, it's usage is removed

[ashish@xeon:/var/www/html/d8]$grep module_uninstall -r *
core/includes/module.inc:function module_uninstall($module_list = array(), $uninstall_dependents = TRUE) {

Change Record is Here : - https://www.drupal.org/node/1894902

CommentFileSizeAuthor
#18 remove_module_install-2352917-18.patch1.26 KBJeroenT
#6 interdiff.txt684 bytesjoshi.rohit100
#6 remove-module_uninstall-2352917-6.patch1.21 KBjoshi.rohit100
#1 remove-module_uninstall-2352917-1.patch811 byteschintan.vyas
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chintan.vyas’s picture

chintan.vyas CreditAttribution: chintan.vyas commented
FileSize
remove-module_uninstall-2352917-1.patch811 bytes

Removed the module_uninstall function which is not required anymore. Tested installing book module and uninstalling it and book table is getting deleted when I uninstall the module.

chintan.vyas’s picture

chintan.vyas CreditAttribution: chintan.vyas commented
Assigned: Unassigned » chintan.vyas
chintan.vyas’s picture

chintan.vyas CreditAttribution: chintan.vyas commented
Assigned: chintan.vyas » Unassigned
Status: Active » Needs review
a_thakur’s picture

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

Patch works fine.

[ashish@xeon:/var/www/html/d8]$grep module_uninstall -r *
[ashish@xeon:/var/www/html/d8]$

Changing to RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Needs work

Can we remove module_install in this issue too and link the correct change record?

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Status: Needs work » Needs review
FileSize
remove-module_uninstall-2352917-6.patch1.21 KB
interdiff.txt684 bytes

Removed the module_install as well

Status: Needs review » Needs work

The last submitted patch, 6: remove-module_uninstall-2352917-6.patch, failed testing.

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented
Issue summary: View changes
joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented

Test is failing due to drush.

dinarcon’s picture

dinarcon CreditAttribution: dinarcon commented
Title: Remove module_uninstall() from module.inc » Remove module_install() and module_uninstall() from module.inc
Status: Needs work » Needs review

Updating issue's name based on @alexpott comment in #5. Queue for retesting.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

We need the new testbot infrastructure that is not dependent on drush to proceed with this change - or drush to be fixed.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
CreditAttribution: tstoeckler commented

So let's proceed with #1 in the meantime?

a_thakur’s picture

a_thakur CreditAttribution: a_thakur commented

Agree with tstoeckler. Let go with #1 in the meantime, a different issue can be created to remove module_uninstall().

joshi.rohit100’s picture

joshi.rohit100
he/him
Primary language Hindi
Location Delhi, India
CreditAttribution: joshi.rohit100 commented

For module_install(), issue is already created but drush problem is also there.
Here https://www.drupal.org/node/2301605

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented

Having module_install() but not module_uninstall() does not make a lot of sense. Rather than discussing things here we should either but putting our efforts into the new testbot architecture or fixing drush to not use module_install for d8.

JeroenT queued 6: remove-module_uninstall-2352917-6.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 6: remove-module_uninstall-2352917-6.patch, failed testing.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented
Status: Needs work » Needs review
FileSize
remove_module_install-2352917-18.patch1.26 KB
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT commented

Assigned to the following CR: Module/hook system functions replaced with module_handler and module_installer service

LinL’s picture

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

No module_install() or module_uninstall() in core other than in module.inc, CR done. Looks good to go.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Reviewed & tested by the community » Fixed

Committed f1d807b and pushed to 8.0.x. Thanks!

  • alexpott committed f1d807b on 8.0.x 
    Issue #2352917 by joshi.rohit100, chintan.vyas, JeroenT: Remove...

Status: Fixed » Closed (fixed)

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