Closed DominicDetta closed 9 months ago
I'm not the maintainer, so just a few non-normative comments:
Clockwork\Support\Vanilla\Clockwork
? First off, the additional utility functions (isEnabled()
in particular) are really useful additions, but I'd just put them into the baseclass. Then, only the constructor remains, where I find it's really bad practice to not invoke parent::__construct()
. I think, just using the enhanced vanilla baseclass would work just as well, so that class and config.php can go./latest
, a placeholder for the newest record, and there is /<ID>/extended
, which retrieves full info (in particular the XDebug profiler data) from the according data sources.That said, cool contribution! I personally don't use Mezzio, but open source lives from participation!
Thank you for your fast response.
I derived from Clockwork\Support\Vanilla\Clockwork
because I want to be able to use all built-in functions but the only down-side was that the original config.php was using the $_ENV
which I replaced it with a cross-compatible function getenv
. In this way I guarantee that works in any environment. Furthemore I didnt invoke the parent::__construct()
because some application could configure Clockwork directly using environment variables without passing any PHP configuration.
I didnt want to modify the base class but if approved I can move all utility functions to it.
I verified and the /latest
works correctly. I will try the other missing routing.
I solve the parent__construct()
issue as desired.
Do you know if the project is using the PSR-12 as formatter?
The class Clockwork\Support\Vanilla\Clockwork
is completely changed
I tested also the the routing for /extended
and it works perfectly.
Clockwork seems to use tabs to indent the code. If your IDE autoformatted the files with four-space indent (which is the default per PSR, I believe), then it breaks the formatting here.
Hey, thanks for working on this.
Would you be interested in releasing this integration on your own as a separate package, with me linking it in the official Clockwork readme/website?
Could you please fix the indentation to use tabs? Otherwise the diffs are impossible to read. Also, what is the meaningful difference between using $_ENV
and getenv
?
Yes, I can create a separate package.
The difference between $_ENV
and getenv
is obscure to me as well but from my experience in a docker environment with Apache the environment variables are not returned when using $_ENV
.
https://stackoverflow.com/questions/8798294/getenv-vs-env-in-php has some info on that...
https://stackoverflow.com/questions/3780866/why-is-my-env-empty may be even better. With that in mind, I'd vote for using getenv()
exclusively and not using the volatile $_ENV
. That would be a change for the the Vanilla integration though, not just for Mezzio.
It would be preferrable to integrate this PR and avoid to create a separate package as it would be additional installation which would make this project interesting for those who use microframework Mezzio which is a project of Laminas
To make the integration with clockwork more interesting, may I add that I have the intention of requesting an amendment to Mezzio's documentation
Just wondering, does Mezzio come with a PSR-17 response factory, included or as dependency? Point is, using PSR-7, PSR-15 and PSR-17 as base, I think I could build a middleware that can be integrated into pretty much any framework that uses those HTTP PSRs, including Slim (which is my concern) and Mezzio.
When you install Mezzio from the skeleton provided the dependency laminas-diactoros is installed which implement PSR-7 and PSR-17.
Right now my middleware use the Laminas\Diactoros\Response
. If there is a way to create a response without relying to it, it will be completely framework agnostic.
@UlrichEckhardt I was actually thinking the other day, why do we still need a custom Slim integration, when the Vanilla integration is superior and more full-featured. I think it would be a really good idea to add a generic PSR-compatible middleware to the Vanilla integration as you suggest.
@DominicDetta I think I would put this on hold and explore the route of adding a generic middleware to support all these PSR-compatible micro-frameworks first.
Also, I'm okay with changing the Vanilla config file to use getenv
, if anyone wants to make that PR.
https://github.com/UlrichEckhardt/clockwork-slim-demo/tree/feature/psr-17-middleware -- Slim doesn't need a custom integration, it provides implementations for the PSR-{7,15,17} interfaces, so it could use any middleware built on top of that. I've realized that in the above demo project, which should be seen as a proof of concept.
There's still a few things to do:
Middleware
needs to be integrated into the clockwork sources, consistently formatted with tabs.Support\Vanilla\Clockwork
class.@DominicDetta I think I would put this on hold and explore the route of adding a generic middleware to support all these PSR-compatible micro-frameworks first.
Ok, that sounds right.
I just added #680, including two demo projects for both Slim and Mezzio. This is still in draft form, in particular there are still a few places marked TODO: ...
that need work. Still, it's working and something to toy with.
This PR will be replaced by #680
This pull request allow to use Clockwork in Mezzio application.