PSR-0 Autoloading for easier integration

[mandoz]

Namespacing and PSR-0 autoloading for easier integration into frameworks like Laravel.

[fennb]

Hey there, thanks for the contribution!

Couple of questions/notes:

1) This would cause a dependency on PHP>=5.3.0 (which is probably fine)

2) I'd prefer 'fennb' not be in the namespace (though I understand according to github, this would be normal).

3) Out of interest, why is Phirehose.php considered an entirely different file? (ie: there's no normal diff behaviour). This should be avoided if possible as it breaks any concept of git-blame (or history).

4) Is it a requirement to have one-file-per-class? This seems a bit cluttered for things like the Exception sub-classes which are merely there to encapsulate naming/hierarchy and contain no actual functionality.

Sorry, I'm not super familiar with PHP-FIG/PSRs as I don't actively work in PHP these days, so happy to take guidance from the Phirehose community.

Cheers!

[mandoz]

Hello,

1) PHP>=5.3.0 is a conservative requirement anyway. Most modern frameworks have similar or higher requirements. Laravel 5.1 requires a minimum of 5.5.9.

2) I'm afraid it has to be that way since it has to be consistent with your composer package name definition is "fennb/phirehose" and we wouldn't want to rename the package.

3) I believe its because of the PSR-2 fix (which changes whitespaces/tabs etc.) resulting in a high "delta" percentage which pushes git to "recreate" the file.

4) Yes.

Let me know if you have any other questions.

[DarrenCook]

I'm a bit concerned by this patch: changing the file structure is quite radical and is likely to break backwards-compatibility.

Phirehose is for relatively short data collection scripts, run from the commandline - they should not really contain any notable processing, so don't particularly need to be part of a larger framework. I've never written a Phirehose script and wished I had auto-loading: there is just one class, you will always use it, and you are unlikely to need to load another class.

As you've already done the work, my suggestion would be to set up a "friendly fork" (friendly in that the two projects link to each other, and keep an eye on patches), and see what happens. If we find that the important functionality/security patches are coming in on the fork, that tells us it was desirable.

[oneafrikan]

+1

On 22 June 2015 at 11:46, Darren Cook notifications@github.com wrote:

I'm a bit concerned by this patch: changing the file structure is quite radical and is likely to break backwards-compatibility.

Phirehose is for relatively short data collection scripts, run from the commandline - they should not really contain any notable processing, so don't particularly need to be part of a larger framework. I've never written a Phirehose script and wished I had auto-loading: there is just one class, you will always use it, and you are unlikely to need to load another class.

As you've already done the work, my suggestion would be to set up a "friendly fork" (friendly in that the two projects link to each other, and keep an eye on patches), and see what happens. If we find that the important functionality/security patches are coming in on the fork, that tells us it was desirable.

— Reply to this email directly or view it on GitHub https://github.com/fennb/phirehose/pull/86#issuecomment-114063644.

[mandoz]

What would break backwards compatibility would be the Namespacing and not the file structure. If you bring this package in with composer, a composer update will take care of the new file structure and since the new files are PSR-0 autoloaded, the "inclusion" of the file and its path is pretty much abstracted.

I forked this branch and made this patch to actually use it in a Laravel 5.1 project, Which allows you (as other modern frameworks do) to run command line functions that also integrate well with other functionality. There are also microframeworks like Lumen which would make use of a package.

At the end of the day its not about using it within a framework or not, because in both cases, its a good idea to follow modern coding practices (PSRs), and having package classes available in the global namespace is just not a good idea.

[mandoz]

Plus I don't see why a 2.0 release with 1 breaking change (adding the use statement on top of your file would be such a horrible thing to do :) Its a small price to pay for cleaner more organized code more maintainable code.