DaveLiddament / sarb

Static Analysis Results Baseliner
MIT License
162 stars 17 forks source link

replaced webmozart/path-util with symfony/filesystem #107

Closed a4blue closed 2 years ago

a4blue commented 2 years ago

The deprecation notice annoyed me, so i tried to change it :) As far as i can tell, the change shouldn't be too drastic

codecov-commenter commented 2 years ago

Codecov Report

Merging #107 (926440c) into master (72bc655) will decrease coverage by 0.12%. The diff coverage is 33.33%.

Impacted file tree graph

@@              Coverage Diff              @@
##              master     #107      +/-   ##
=============================================
- Coverage     100.00%   99.87%   -0.13%     
- Complexity       501      502       +1     
=============================================
  Files            105      105              
  Lines           1576     1578       +2     
=============================================
  Hits            1576     1576              
- Misses             0        2       +2     
Impacted Files Coverage Δ
src/Domain/Common/AbsoluteFileName.php 100.00% <ø> (ø)
...c/Framework/Command/internal/ProjectRootHelper.php 100.00% <ø> (ø)
src/Domain/Common/ProjectRoot.php 92.59% <33.33%> (-7.41%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 72bc655...926440c. Read the comment docs.

DaveLiddament commented 2 years ago

Thanks for this PR. I'm onboard with the aim of removing the deprecation notice.

Given SARB's use case it will probably be used in with older code bases. I'm trying to keep SARB's dependencies as wide as possible. It's main dependencies are the Symfony components and my aim is for SARB to work with all supported versions of Symfony components (and PHP versions). The restriction to Symfony filesystem 5.4 is too tight. For adding a new Symfony library it would ideally satisfy the version constraints 4.4 || ^5.0 || ^6.0.

To progress to your aim I've created the following issues:

I hope to spend some time over the xmas break to get this and a few of the other outstanding issues sorted.

Again, thanks very much for this PR. I'm sorry I'm not accepting it at the moment. I hope the reasons why make sense, and the plan to meet your ultimate aim is acceptable.

a4blue commented 2 years ago

@DaveLiddament could you specify, which PHP Version you aim to Support ? As per composer.json i think PHP 7.3-8.1 is your target. I also tried to fork the path-utils Project now, so that Projects that need replacement dont diverge too much apart :) https://github.com/a4blue/path-util I hope the fork is acceptable for you to use

DaveLiddament commented 2 years ago

Fixed by #110