Add deprecate messages to JError#16952
Conversation
|
FWIW it's not just removing So 👍 on working on phasing it out. Just keep in mind it's probably not going to be as simple as changing |
|
Most usage of JError are the raise* functions. What you think about the replacement messages? |
libraries/legacy/error/error.php
Outdated
| * | ||
| * @since 1.5 | ||
| * @deprecated 1.7 | ||
| * @deprecated 4.0 Throw an Exception or enqueue the message to the application, eg. \Joomla\CMS\Factory::getApplication()->enqueueMessage($msg) |
There was a problem hiding this comment.
I don't think we can guide people and say "do one of these two things". Not every error needs to lead to a thrown Exception or an enqueued message (I'd say more need to be logged than displayed to the user through the UI myself). So for this method I think it's fine to leave it without a message.
There was a problem hiding this comment.
That's the two functions which will be executed in the Core by default. But I will remove it.
libraries/legacy/error/error.php
Outdated
| * | ||
| * @since 1.5 | ||
| * @deprecated 1.7 | ||
| * @deprecated 4.0 Use \Joomla\CMS\Factory::getApplication()->enqueueMessage($msg, 'warning') instead |
There was a problem hiding this comment.
Again, not every warning or notice needs to lead to a UI message. So I don't think this can be the de facto replacement.
There was a problem hiding this comment.
I'v put messages here according to what the default behavior is in Joomla from framework.php.
There was a problem hiding this comment.
I know that's the behavior, but I think we need to challenge the status quo as it comes to error handling. Some people are going to see these messages and do a blind search/replace. This is something we really should have a documentation page around which describes how different types of error scenarios should be handled and how that ultimately translates into a user being informed of different error cases.
There was a problem hiding this comment.
For error messages we have a document already https://docs.joomla.org/display_error_messages_and_notices. So the only thing we have to document when to throw an exception. I just don't have time to do that as well and this is not really connected to this pr as we want to offer a solution for devs which were using a deprecated api.
There was a problem hiding this comment.
We're getting into much more than a "find uses of this deprecated method, change it to use this method instead" discussion as it relates to error handling though. Because not every error condition is equal.
You're right in making sure the docs are accurate isn't related to this PR, I just don't agree 100% with the "official" guidance in the deprecation message being "find uses of this deprecated method, change it to use this method instead".
There was a problem hiding this comment.
I guess if there are devs which are using JError::raiseWarning then they expect a same behavior as replacement which is enqueueMessage. So what do you suggest then to write for a deprecated message?
There was a problem hiding this comment.
If you're doing a one-for-one replacement, then yes. But I honestly don't believe that every call to JError::raiseWarning() necessarily translates into needing a notification to the UI. That's the status quo I'm trying to challenge.
There was a problem hiding this comment.
I understand that and I hope, when somebody sees that doc block that it makes her/him rethinking what exactly he/she want's to do with the warning. But at least we should give some guidance.
I changed the docblock the wait that enqueueMessage can be a solution to notify the UI.
|
Set to "closed" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/16952 |
|
closed as having PR #17013 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16952. |
|
This one should stay open. It is for version 3 which improves the deprecated messages. |
|
Set to "open" on behalf of @franz-wohlkoenig by The JTracker Application at issues.joomla.org/joomla-cms/16952 |
|
reopened, thanks for Hint, @laoneo This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/16952. |
|
Any plan with this one? |
4 participants
The
JErrorclass is deprecated since Joomla 1.7. Guess it is time to say good by for it in Joomla 4. If this pr get accepted, then I will remove all the JError calls in the 3 series, that we can safely remove the class in 4. But first I want to have an agreement if the replacements are so ok for the maintainers.