The extension polyfill may be split from the SDK

blackfireio / php-sdk

The Blackfire PHP SDK

https://blackfire.io

MIT License

150 stars 22 forks source link

The extension polyfill may be split from the SDK #4

Closed stof closed 7 years ago

stof commented 8 years ago

Currently, the SDK contains 2 things:

the SDK itself, requiring PHP 5.3+ because of the usage of namespaces (hence my PR #2)
the ext-blackfire polyfill, implementing BlackfireProbe for PHP 5.2+ on top of XHProf (well, except that autostart.php uses __DIR__, killing the 5.2 support. this is fixed by #9)

It may be a good idea to split them in separate repos.

And btw, if they stay in the same repo, the check in https://github.com/blackfireio/php-sdk/blob/5720c3047220dec183e522d90e3daf73371f8037/src/Blackfire/Client.php#L28 can never be false

lyrixx commented 8 years ago

:+1:

nicolas-grekas commented 8 years ago

:-1: it would complicate things. composer doesn't work on 5.2 so 5.2 users have and always will have to install by their own mean.

stof commented 8 years ago

@nicolas-grekas then, what do you think about #2 ?

Hywan commented 8 years ago

@lyrixx What if we test the SDK with atoum, do we have a chance to see our PR merged? /cc @jubianchi, @agallou, @mageekguy

fabpot commented 8 years ago

@Hywan Thanks for the proposal but we need tests written in PHPUnit as this is the library we are using everywhere else (we don't have any experience with Atoum).

Hywan commented 8 years ago

@fabpot I understand. We were planning to use the official atoum's blackfire-extension to test the extension itself, this would have been a good demonstration. Nevermind, thanks!