-
Notifications
You must be signed in to change notification settings - Fork 264
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
Use Symfony\Filesystem for file operations #83
Comments
For the amount of filesystem operations this library does I'm not sure requiring another library would be beneficial. Wouldn't SplFileInfo and SplFileObject work instead? http://www.php.net/manual/en/spl.files.php |
Yeah. After digging in and using it a couple weeks ago I realized that Symfony\Filesystem doesn't really buy us that much. We can drop it as a dependency. |
Then why it hasn't been dropped yet?... This is a nice light-weight image lib (just what i was looking for), but now I must alter it to use. It's completely unnecessary to bring in a lib dependence for a single file existence check. |
Yup. Which is why we said we'll drop it. Feel free to submit a pull request if you have time to work on it. |
I have same problem with the thing. I think phpthumb should use is_file instead of symfony filesystem. Why you don't use built-in function, that is better ? |
Again, feel free to submit a pull request fixing this. I don't have time to work on this library anymore. :( |
A pull request for three lines of changes? Here are the changes to make it easy: remove line 62. change what is now line 91 to read: Remove the requirement from Composer.json. |
I created a pull request #127 |
This has been on my mind for some time but this issue brought it to my attention a little harder. The issue is for the 1.0 branch but it effects 2.0 as well.
Instead of rolling our own filesystem code, I propose use a library specifically designed to handle these sorts of things. That way PHPThumb is focused on images, and not filesystem operations.
Also, abstracting out our operations will allow us to mock those operations, enhancing our unit testing capabilities.
The text was updated successfully, but these errors were encountered: