-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
Absolute vs relative paths #156
Comments
@Potherca Would you mind pitching in with an opinion please ? |
Working on a response, to be updated soon (need to catch the bus now 🚌🏃💨) |
Okay,so... I was trying to remember why relative paths were initially used (other than for convenience) and "how we got here"™️. One path was through the MRs/Issues linked above, the other was through the commit log. The current state of afairs (absolute paths for global install, relative path for local install) comes from the use-case outlined in #14.
and
The user feedback basically gives two competing problems/solutions:
🆚
So this got me thinking about what the uses-cases and scenarios there actually are. I think this one:
is the most relevant, as this is the most common problem (also including docker and running the plugin in CI/CD environments like Gitlab Pipelines and GitHub Actions). Still need to think about this some more, thought. 🤔 |
Just to be complete, we had another report of things not working due to duplicate paths (one relative, one absolute) yesterday: PHPCompatibility/PHPCompatibility#1303 (comment) |
Thinking about the usecase...
But multi-machine is only when two (or more) environments use the same directory at the same time. The remote CI/CD for instance is still a single-machine. What I think I am trying to say is that a user is unlikely to accidentally have a multi-machine setup (so we need a better description for this). The majority of use-cases I personally encounter in that area are people using docker (or other VMs) and running PHP (or other) commands outside the VM. Which they should not do but it still happens. I get the feeling that for the majority of the usecases using absolute paths would be fine, but we don't know what the users wants or needs. So they need to tell us. Which leads back to the solution originally proposed in #14 of adding a config setting. A That should also give us more time to gather usecases and decide if moving all path to absolute by default is then still needed. |
@Potherca Thanks for adding your perspective. While I agree that adding the config settings would be nice addition, I have a feeling that it doesn't address the real-life problems people currently have with duplicate paths. |
☝️ Turns out that the duplicate path reported in PHPCompatibility/PHPCompatibility#1303 (comment) was not caused by this plugin, but by the user also running a script. |
I think we might still want to consider adding the flags to force absolute or relative paths, to give users more control... |
@Potherca Oh, totally! And the fact that the above mentioned issue was not caused by the plugin, doesn't take anything away from the results I reported in the original description where I was seeing duplicate paths being registered during tests runs (for tests not yet pulled), so the original issue still stands. |
I'd like to re-open the discussion around whether the paths registered with PHPCS should be absolute or relative paths.
Some previous issues related to this: #14, #33, #73
Current status after #28:
Problems I'm seeing
Proposal
Always use absolute paths.
Further research needed
vendor
directory ?=> I'd like to suggest that they can add a script to run this plugin to their
composer.json
and can run that script whenever the project is checked out. After all, the plugin will update the paths based on the runtime environment, so should fix the paths in the committedCodeSniffer.Conf
file to the new environment.=> In Composer 2.x, Plugins can add their own commands to Composer. We could consider adding a custom command to run this plugin. This would negate the need to add the script.
Ref: https://getcomposer.org/doc/articles/plugins.md#command-provider
=> Along the same lines as the search-depth option, we could consider adding a
force-relative-paths
option.We should probably also investigate if custom commands can take custom arguments. In that case, we could possible add support for a
--force-relative-paths
CLI argument if needs be.The text was updated successfully, but these errors were encountered: