Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.0] Remove obsolete uninstallEosPlugin method from script.php #40711

Conversation

richard67
Copy link
Member

@richard67 richard67 commented Jun 3, 2023

Pull Request for Issue # .

Summary of Changes

This pull request (PR) removes the "uninstallEosPlugin" function from script.php and the one call to in the "update" function of the same file.

The function is obsolete in the 5.0 core because it handles uninstallation of the 3.10 EOS plugin, which is already uninstalled when updating from 3.10 to 4.x, and beginning with 4.4 and 5.0 there is a permanently installed EOS plugin which has been introduced with PR #40565 so it doesn't need to keep that function.

Possible B/C break to be approved by release managers

In opposite to previously removed, obsolete code in script.php for 5.0, this time the removed function is not private but protected.

That means that theoretically the method could be overridden in the installation scripts of 3rd party extension developers with code for doing own stuff when their scripts is based on the "JoomlaInstallerScript" class of our script.php.

But the name of that method is so specific to the core that I don't really think anybody has ever done that.

Testing Instructions

Code review.

Actual result BEFORE applying this Pull Request

File "administrator/components/com_admin/script.php" contains the obsolete function "uninstallEosPlugin" which is called only at one place in the public function "Update" of the same file.

Expected result AFTER applying this Pull Request

File "administrator/components/com_admin/script.php" doesn't contain anymore the obsolete function "uninstallEosPlugin" and any call to it.

Link to documentations

Not sure if it needs to be documented somewhere for the "JoomlaInstallerScript" class on manual.joomla.org that this method has been removed.

Please select:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@sandewt
Copy link
Contributor

sandewt commented Jun 3, 2023

Should / can the language strings of the eos310 plugin also be removed yet for J5? What is the procedure for this?

See:

; Below is a copy of the 3.10 text definitions for reference.

@richard67
Copy link
Member Author

Should / can the language strings of the eos310 plugin also be removed yet for J5? What is the procedure for this?

See:

; Below is a copy of the 3.10 text definitions for reference.

I think that either can be done in 4.4-dev and then be merged up into 5.0-dev because they are obsolete and only used by the core. But if we are strict regarding removal of language strings we might have to deprecate them now and remove with 6.0.

=> @HLeithner Any suggestions?

@sandewt
Copy link
Contributor

sandewt commented Jun 3, 2023

I have tested this item ✅ successfully on 1fd0b2d


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40711.

@HLeithner HLeithner added b/c break This item changes the behavior in an incompatible why. HEADS UP Removal Removes functionality labels Jun 5, 2023
@HLeithner HLeithner merged commit d1efd5b into joomla:5.0-dev Jun 7, 2023
@HLeithner
Copy link
Member

Thanks

@richard67 richard67 deleted the 5.0-dev-remove-eos310-plugin-uninstallation-on-update branch June 7, 2023 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
b/c break This item changes the behavior in an incompatible why. HEADS UP Removal Removes functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants