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

Global enable / disable, very basic FirePHP compatibility #55

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

ErikKrause
Copy link

Implemented a global enable / disable function and a setting to switch on or off FirePHP compatibility (defaults to off). If switched on argument order is changed if exactly 2 arguments are passed to the various log functions to be able to re-use old FirePHP calls.

Added global enable / disable, added setting for FirePHP log style. If switched on argument order is changed such, that you can reuse old firephp calls.
Added global enable / disable, added setting for FirePHP log style. If switched on argument order is changed such, that you can reuse old firephp calls.
json_encode failed for resources causing an empty log. Now uses print_r and get_resource_type for resource, print_r for all other.
@DanMan
Copy link

DanMan commented Feb 2, 2017

Coming from FirePHP myself this is generally a good idea, but you changed a bit more than necessary, which I think is inappropriate. Maybe just add the switch and leave the rest up to the maintainer?

@ghost
Copy link

ghost commented Feb 2, 2017

Well, I changed exactly what was needed to make it work for me. Said that I'd like to know what exactly you think is more than necessary.

@Asenar
Copy link

Asenar commented Feb 14, 2017

Hi @Erik-Krause / ( @ErikKrause ?) thanks you very much for your adds !

Maybe you can change your composer.json to set the name as erikkrause/composerphp and register it in composer ?

@hopeseekr
Copy link

Whenever you fork a project via GitHub and you maintain its master branch way after you make a PR on the original PR, this is what happens. That's why I literally deleted my fork and just created a standalone repo.

@hopeseekr
Copy link

@Erik-Krause Can you prepare a PR for PHPExpertsInc/chromephp#3? I don't want the composer.json changes. I don't want all those (ek) comments, but your copyright clause comments are very good and well, especially since the Apache v2 license pretty much requires attribution.

@ErikKrause
Copy link
Author

What do you mean by "all those (ek) comments". I won't remove the FirePHP compatibility changes, since that's the whole point. Apart of that I like your proposal to move this to a new repo.

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.

7 participants