Skip to content

Update framework #48

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

Merged
merged 12 commits into from
Jul 11, 2025
Merged

Update framework #48

merged 12 commits into from
Jul 11, 2025

Conversation

rdeutz
Copy link
Contributor

@rdeutz rdeutz commented Jul 8, 2025

see commits

@rdeutz rdeutz changed the base branch from 3.x-dev to 4.x-dev July 8, 2025 16:29
@rdeutz
Copy link
Contributor Author

rdeutz commented Jul 9, 2025

Couple of things here:

  • Since phpunit10 E_* messages are no longer converted to execptions so our testing had to be ajusted
  • We said in our testing that we check for execptions but in real we didn't
  • My opinion is that an execption would be the right thing to do so I changed the package to throw execptions when a call to a non existing function is made
  • I did the same for __get but it might also be acceptable when we return an empty array if nothing matches.
  • I have thrown Execption::class because I didn't want to add new execptions classes to the package. I think we shoudl create a execption package to serve other parts of the system with a set of more specific exections.

Other opinions?

@Hackwar
Copy link
Contributor

Hackwar commented Jul 9, 2025

* My opinion is that an execption would be the right thing to do so I changed the package to throw execptions when a call to a non existing function is made

Agreed

* I did the same for __get but it might also be acceptable when we return an empty array if nothing matches.

I disagree here. We had this in v2/3 in the application and I think that is an acceptable solution: https://github.com/joomla-framework/application/blob/3.x-dev/src/AbstractWebApplication.php#L240
Returning an empty array seems wrong in any case.

rdeutz and others added 2 commits July 11, 2025 14:34
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@rdeutz
Copy link
Contributor Author

rdeutz commented Jul 11, 2025

RTC

@richard67 richard67 merged commit 7f8385e into 4.x-dev Jul 11, 2025
12 checks passed
@richard67 richard67 deleted the update-framework branch July 11, 2025 13:06
@richard67
Copy link
Contributor

Thanks.

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.

4 participants