PR-4.4-dev Pending

User tests: Successful: Unsuccessful:

avatar Fedik
Fedik
20 Sep 2023

Pull Request for Issue # .

Summary of Changes

The PR changes pre-processing methods from setFobar to onSetFoobar for existing event classes.

This is follow up for

The PR can be merged only when that PR is accepted and merged.

Testing Instructions

Code review.
Make sure all works as before.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:
  • No documentation changes for docs.joomla.org needed
  • Pull Request link for manual.joomla.org:
  • No documentation changes for manual.joomla.org needed
avatar joomla-cms-bot joomla-cms-bot - change - 20 Sep 2023
Category Libraries
avatar Fedik Fedik - open - 20 Sep 2023
avatar Fedik Fedik - change - 20 Sep 2023
Status New Pending
avatar Fedik Fedik - change - 20 Sep 2023
The description was changed
avatar Fedik Fedik - edited - 20 Sep 2023
avatar Fedik Fedik - change - 20 Sep 2023
Labels Added: PR-4.4-dev
avatar HLeithner
HLeithner - comment - 20 Sep 2023

pretty sure we need documentation here and for the other pr, especially for the deprecated stuff

avatar Fedik
Fedik - comment - 20 Sep 2023

It is there

avatar HLeithner HLeithner - change - 20 Sep 2023
Status Pending Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2023-09-20 16:13:55
Closed_By HLeithner
avatar HLeithner HLeithner - close - 20 Sep 2023
avatar HLeithner HLeithner - merge - 20 Sep 2023
avatar HLeithner
HLeithner - comment - 20 Sep 2023

merged because it's the follow up pr of #41722 thanks

avatar laoneo
laoneo - comment - 21 Sep 2023

Should the calls not be vice versa? That the deprecated method is calling the new one?

avatar Fedik
Fedik - comment - 21 Sep 2023

When method onSetFoobar exists it will be called first and then fallback to setFoobar for b/c.
If we do deprecated method is calling the new one, then deprecated method will be never called

avatar laoneo
laoneo - comment - 21 Sep 2023

I mean should the code not be like:

    public function setUser(User $value): User
    {
        return $this->onSetUser($value);
    }

    protected function onSetUser(User $value): User
    {
        return $value;
    }

Normally you deprecate a function and then in the function you forward to the new one. But perhaps I miss here something.

avatar Fedik
Fedik - comment - 21 Sep 2023

But perhaps I miss here something.

This:

When method onSetFoobar exists it will be called first

in your example setUser will never be called. Because onSetUser exists, and will be called first.

avatar laoneo
laoneo - comment - 21 Sep 2023

Ok, don't have time to dig here deeper. I guess you know what you are doing. Looks all a bit contrary to what we do normally when we deprecate stuff, that's why I commented here.

Add a Comment

Login with GitHub to post a comment