Skip to content
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

Drastically simplify using Composer Runtime API #64

Closed
wants to merge 11 commits into from
Closed

Conversation

weitzman
Copy link
Collaborator

@weitzman weitzman commented Nov 11, 2023

I think this is all thats needed now that Composer provides a Runtime API to get the paths we want.

  • No longer dependant on authors customizing their composer.json
  • Comes with tests.
  • This should be a new major version.

@weitzman
Copy link
Collaborator Author

This PR was motivated by DrupalFinder not working at ddev/ddev-drupal-contrib#14 (composer.json not customized, on purpose)

@mglaman
Copy link

mglaman commented Nov 13, 2023

I was converging on this, as well, for phpstan-drupal. I think it's safe to say everyone is on the correct version of Composer, which added this. It's been two or three years, maybe more?

It'd be great to have this merged and kick off the 2.x of the package.

src/DrupalFinder.php Outdated Show resolved Hide resolved
src/DrupalFinder.php Outdated Show resolved Hide resolved
src/DrupalFinder.php Outdated Show resolved Hide resolved
composer.json Show resolved Hide resolved
@webflo
Copy link
Owner

webflo commented Nov 15, 2023

I have extended the test coverage and added support for custom vendor-dir. This is rarely used, but we should nevertheless have it supported.

Added a test-case for "default" Drupal with customized path in extra.installer-paths.

@webflo
Copy link
Owner

webflo commented Nov 15, 2023

Are there more obscure test cases?

@mglaman
Copy link

mglaman commented Nov 15, 2023

I think this is OK. Custom vendor directory should automatically be handled in the compiled runtime data, as Composer installed its information there.

Copy link
Collaborator Author

@weitzman weitzman left a comment

Choose a reason for hiding this comment

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

Looks even better. Thanks Webflo.

@webflo
Copy link
Owner

webflo commented Dec 6, 2023

The README file needs work. The purpose of this library has changed with this PR. It no longer uses the "current path" or filesystem.

I was wondering if we should keep the old DrupalFinder class around, and rename it to DrupalFinderFilesystemDiscovery or something similar. The composer runtime approach works for tools within the shared autoloader. DrupalFinderFilesystemDiscovery should be used to locale a Drupal installation in a generic way.

@weitzman
Copy link
Collaborator Author

weitzman commented Dec 7, 2023

I personally would recommend the 1.x branch for non composer tools.

@weitzman
Copy link
Collaborator Author

Updated README.

@bimsonz
Copy link

bimsonz commented Feb 16, 2024

  • 🚀 would love to see this get in

@tyler36
Copy link

tyler36 commented Mar 22, 2024

Hit this while trying to add mglaman/phpstan-drupal to a local module that uses ddev/ddev-drupal-contrib

@webflo
Copy link
Owner

webflo commented May 8, 2024

Merged #65 instead

@webflo webflo closed this May 8, 2024
@webflo webflo deleted the runtime branch May 8, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants