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
mvc - larger overhaul replacing Phalcon Application #6389 #7455
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it very much, great work! left some notes for future changes perhaps.
27589f0
to
b77d970
Compare
This removes most phalcon code currently being used on our end, except the Volt templates (which are re-wrapped) and a translation class we can easily replace later in a separate commit. Consumers of our controller classes shouldn't notice a difference as the used objects and methods are named the same. The most notable changes are the following ones: * Exceptions about not being able to find a requested path now break down into different exceptions inheriting from DispatchException, which makes it easier from the entrypoint (api.php, index.php) to catch and handle accordingly. * When not in development mode, raw exceptions are not being returned anymore, which increases security * The Dispatcher class is reponsible for object construction and mapping validation (valid uri, but no object found) * The Router class replaces previous Application class, it disects offered uri's into namespaces, classnames and methods to call. In the long run there should be a seperate controller for controllers using volt templates or api calls, but as the existing ones don't distinct between this and the output handling is different now, we can park this for a later moment in time (the performance penalty should be rather low). Some unused functionality has been removed, for example support for the X-HTTP-Method-Override header in Request->getMethod() (see https://github.com/phalcon/cphalcon/blob/44243c07658d060cd8a21761743b0f4fc01641aa/phalcon/Http/Request.zep#L599-L609).
b77d970
to
96d00a2
Compare
Merged, thanks! 🥳 |
@fichtner I understand this is probably not the best place to ask this, so I apologize upfront -- but why is Phalcon being removed? I'm not really aware of what's happening in PHP community but I'm curious to know why it's being considered frowned upon. |
@kriansa lowering the dependency chain and ease of maintenance in the long run. For us Phalcon doesn't add much value and over the years we spend quite some time handling quirks which would have been easier to solve without the framework in between. We will keep some of it (the volt templates), due to lack of choices in that area. |
@kriansa fair question, don't worry. I think at some point we might even break out volt into its own module so we are not bogged down waiting for Phalcon to support a newer PHP version in the long-term https://github.com/phalcon/volt |
Thanks guys, I appreciate the transparency 👍 |
(#6389)
This removes most phalcon code currently being used on our end, except the Volt templates (which are re-wrapped) and a translation class we can easily replace later in a separate commit. Consumers of our controller classes shouldn't notice a difference as the used objects and methods are named the same.
The most notable changes are the following ones:
In the long run there should be a seperate controller for controllers using volt templates or api calls, but as the existing ones don't distinct between this and the output handling is different now, we can park this for a later moment in time (the performance penalty should be rather low).
Some unused functionality has been removed, for example support for the X-HTTP-Method-Override header in Request->getMethod() (see https://github.com/phalcon/cphalcon/blob/44243c07658d060cd8a21761743b0f4fc01641aa/phalcon/Http/Request.zep#L599-L609).