Skip to content

replaced path-util with symfony/filesystem #59

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

replaced path-util with symfony/filesystem #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
],
"require": {
"php": "^7.3 || ^8.0.0",
"webmozart/path-util": "^2.2"
"symfony/filesystem": "^5.4"
Copy link
Collaborator

@Ocramius Ocramius Dec 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit vary on introducing symfony/* dependencies, since they are inherently unsafe from a dependency PoV:

https://github.com/symfony/filesystem/blob/731f917dc31edcffec2c6a777f3698c33bea8f01/composer.json#L19

See also #57

Copy link
Author

@a4blue a4blue Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is at least one other Project that does not want to use the symfony Project for the same reasons, i tried to Fork it and update it in this Project https://github.com/a4blue/path-util
If i require it instead, i think it should be ok with everyone :)

Copy link
Member

@webmozart webmozart Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Ocramius What's the stance of the Symfony team on that?

Copy link
Collaborator

@Ocramius Ocramius Dec 9, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea: tbh, including their classes (renamed, with a licence notice here) is probably the safest bet.

Copy link

@dereuromark dereuromark Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now merging and releasing this is still better than keeping the abandoned repo in there as dependency.
Along the lines quite a few libraries now generate the warning report here:

Package webmozart/path-util is abandoned, you should avoid using it. Use symfony/filesystem instead.

Copy link

@dereuromark dereuromark Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But in that case a higher support for also Symf 4/5/6 might be good.

Or indeed the classes are just copied - as long as you keep the original copyright (license header), it will be fine.
Thats even easier and will have less dependency issues with symfony and changing their signature/code.

Copy link
Collaborator

@Ocramius Ocramius Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right now merging and releasing this is still better than keeping the abandoned repo in there as dependency.

Not really a problem until a security issue occurs.

I suggest sending a patch with the necessary upstream dependency code, license headers and tests included.

},
"require-dev": {
"symfony/filesystem": "^5.3",
"phpunit/phpunit": "^9.5"
},
"autoload": {
Expand Down
Loading