Skip to content

Conversation

@heelc29
Copy link
Contributor

@heelc29 heelc29 commented Oct 8, 2023

Summary of Changes

deprecate $app and $db property in ActionLogPlugin and their possible use replaced

I'm not sure about the horizon (J6.0 or J7.0) of the removal of the properties

Testing Instructions

check action logs are still working

Actual result BEFORE applying this Pull Request

deprecation messages are logged

@trigger_error('The application should be injected through setApplication() and requested through getApplication().', E_USER_DEPRECATED);

@trigger_error('The database should be injected through the DatabaseAwareInterface and trait.', E_USER_DEPRECATED);

Expected result AFTER applying this Pull Request

deprecation messages are still logged, but properties are no longer used and now themselves deprecated

Link to documentations

Please select:

@laoneo
Copy link
Member

laoneo commented Oct 8, 2023

If you do it in 5.0,then the removal must be 7.0.

@heelc29
Copy link
Contributor Author

heelc29 commented Oct 8, 2023

So the code to set the properties (in CMSPlugin) is not removed before 7.0?

@heelc29 heelc29 marked this pull request as ready for review October 24, 2023 16:06
@HLeithner
Copy link
Member

So the code to set the properties (in CMSPlugin) is not removed before 7.0?

yes next+1 major version

@HLeithner
Copy link
Member

also can you please create a deprecation entry in https://manual.joomla.org/migrations/50-51/new-deprecations

heelc29 added a commit to heelc29/joomla-manual that referenced this pull request Dec 18, 2023
@heelc29
Copy link
Contributor Author

heelc29 commented Dec 18, 2023

also can you please create a deprecation entry in https://manual.joomla.org/migrations/50-51/new-deprecations

Done. joomla/Manual#210

@laoneo
Copy link
Member

laoneo commented Dec 19, 2023

So the code to set the properties (in CMSPlugin) is not removed before 7.0?

If we will do that, then we still can copy the code to this class to support the properties till 7.0. But I doubt that we go to remove the code in CMSPlugin in 7.0.

@HLeithner
Copy link
Member

@laoneo there is no reason to not merge this right?

@bembelimen bembelimen self-assigned this Mar 2, 2024
@bembelimen
Copy link
Contributor

I have tested this item ✅ successfully on 4fbb778


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

@laoneo
Copy link
Member

laoneo commented Mar 4, 2024

I have tested this item ✅ successfully on 4fbb778

Tested with the action log plugin of DPCalendar.


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

@LadySolveig LadySolveig merged commit 2b1b326 into joomla:5.1-dev Mar 4, 2024
@LadySolveig
Copy link
Contributor

Thank you @heelc29

@heelc29 heelc29 deleted the 5.1/deprecations/actionlogplugin branch March 4, 2024 15:25
@Quy Quy added this to the Joomla! 5.1.0 milestone Mar 4, 2024
HLeithner added a commit to joomla/Manual that referenced this pull request Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants