search

bvanhoekelen / performance

⏱ PHP performance tool analyser your script on time, memory usage and db query. Support Laravel and Composer for web, web console and command line interfaces.
Apache License 2.0
520 stars 37 forks source link

please remove the larapack/dd dependency #7

Closed vesper8 closed 6 years ago

vesper8 commented 6 years ago

Your inclusion of this dependency conflicts with the usage of the much more useful https://github.com/kint-php/kint

And furthermore Laravel already ships with the dd() helper https://laravel.com/docs/5.5/helpers#method-dd

So adding it in your package makes it "very opinionated" and problematic

otherwise awesome tool.. just what I was looking for.. thanks!

bvanhoekelen commented 6 years ago

Thanks for your comment and support!

De PHP Performance tool can be used with or without the Laravel framework. This is the reason why i added the larapack/dd dependency. The dependency allow to use dd() outsidte the Laravel framework.

I still like to use the larapack/dd dependency unless it give a error. If you have a error with using the larapack/dd dependency? Can you send the error to me?

At this moment i can do two things: 1: If there are no errors, I can close the issue 2: Remove the larapack/dd dependency and use PHP exceptions or echo with die() instead.

Which option would you choose?

vesper8 commented 6 years ago

It conflicted with my usage of https://github.com/kint-php/kint.. it overrode it basically..

So I had to create my own fork and remove the dependency because I use https://github.com/kint-php/kint 100x per day and it's so much more useful than larapack/dd

Anyway I just don't think larapack should be a dependency.. just like I don't think packages should force a package like https://github.com/kint-php/kint on users.. even though it's extremely useful

I mean think about it.. dd stands for debug and die.. which means that it should not even be left in the code unless commented out.. throwing exceptions would be more appropriate rather than your current usage of dd() IMHO

bvanhoekelen commented 6 years ago

I have some great news for you!

The use of dd and dump was really unnecessary, so i remove the larapack/dd dependency. Now it will no longer conflicted with the kint-php. See version v2.3.2 for more information.

Thanks for your feedback If you have any suggestions please let know

vesper8 commented 6 years ago

thank you!

btw really enjoying your https://github.com/bvanhoekelen/terminal-style :)

I could argue that this dependency also shouldn't be included in this package... but it doesn't interfere with anything and I wouldn't have found out about it otherwise :)