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.1] Followup convert mod_messages to service provider #42735 #42906

Merged
merged 1 commit into from
Feb 29, 2024

Conversation

heelc29
Copy link
Contributor

@heelc29 heelc29 commented Feb 27, 2024

Followup Pull Request for #42735 .

Summary of Changes

Fix the return value in case the exception is caught

Testing Instructions

code review

Old code -> count empty array -> 0:

} catch (RuntimeException $e) {
$messages = [];
// Still render the error message from the Exception object
$app->enqueueMessage($e->getMessage(), 'error');
}
$countUnread = \count($messages);

@Quy
Copy link
Contributor

Quy commented Feb 27, 2024

I have tested this item ✅ successfully on fcb9c2b


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

@ceford
Copy link
Contributor

ceford commented Feb 28, 2024

I have tested this item ✅ successfully on fcb9c2b

Code review - looks fine to me.


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

@Quy
Copy link
Contributor

Quy commented Feb 28, 2024

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Feb 28, 2024
@bembelimen bembelimen merged commit e3000bb into joomla:5.1-dev Feb 29, 2024
4 checks passed
@bembelimen
Copy link
Contributor

Thank you!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Feb 29, 2024
@bembelimen bembelimen added this to the Joomla! 5.1.0 milestone Feb 29, 2024
@heelc29 heelc29 deleted the 5.1/followup-42735 branch March 2, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants