overtrue / phplint

:bug: A tool that can speed up linting of php files by running several lint processes at once.
MIT License
988 stars 89 forks source link

Use non-absolute paths in cache #198

Closed crabmusket closed 11 months ago

crabmusket commented 11 months ago

New Feature

Problem

I run phplint in CI, and would like to cache across runs. However, the cache uses absolute file paths, which may be different on each run.

Suggested solution

I propose adding an option to provide a "source base path", and all file paths will be treated as relative to the base path when being added to the cache.

I'm happy to implement this feature!

llaville commented 11 months ago

Hello @crabmusket

To my opinion, using phplint in CI with caching feature enabled is a nonsense ! I'm 👎 to your request. But I'm not alone. And especially because I'm not the owner of this project (/cc @overtrue ).

overtrue commented 11 months ago

@crabmusket Are there any scenarios where using absolute paths conflicts with caching? I think it should be very easy to do using absolute paths even if you go to validate on a different machine, could you please describe the scenario?

overtrue commented 11 months ago

If there is indeed an inconvenience caused by the path, feel free to PR.

llaville commented 11 months ago

Alternative to change current behavior (absolute/relative path in cache system) is to use dependency-injection system by providing an instance of the Cache : see https://github.com/overtrue/phplint/blob/9.0/src/Linter.php#L68

In this case, you may be able to change the current cache system without to change the core of the linter

llaville commented 11 months ago

@crabmusket You don't reply yet to @overtrue since a week. As the next version 9.1.0 is almost ready for an official release, I would like to know if we should or not work on an alternative solution ?

crabmusket commented 11 months ago

Sorry for the very slow reply! I did some more investigating and realised that actually in our CI builds the filepath remains consistent - it's possible I was seeing cache misses for other reasons. I'll close this for now and maybe come back to it if it continues to be a problem.

use dependency-injection system by providing an instance of the Cache

This is a great tip, thanks!