composer-stager icon indicating copy to clipboard operation
composer-stager copied to clipboard

Evaluate dev dependencies

Open TravisCarden opened this issue 2 years ago • 6 comments

Evaluate dev Composer dependencies according to the Core dependency release cycles, security information, and evaluation criteria for Drupal. This only includes dependencies that are not already a part of drupal/core, drupal/core-dev, or drupal/coder.

Needs review

  • [ ] https://github.com/php-tuf/composer-stager/issues/81
  • [ ] https://github.com/php-tuf/composer-stager/issues/80
  • [ ] https://github.com/php-tuf/composer-stager/issues/297
  • [ ] https://github.com/php-tuf/composer-stager/issues/88
  • [ ] https://github.com/php-tuf/composer-stager/issues/84
  • [ ] https://github.com/php-tuf/composer-stager/issues/296

Removed

  • [x] https://github.com/php-tuf/composer-stager/issues/86
  • [x] https://github.com/php-tuf/composer-stager/issues/82
  • [x] https://github.com/php-tuf/composer-stager/issues/87
  • [x] https://github.com/php-tuf/composer-stager/issues/89
  • [x] https://github.com/php-tuf/composer-stager/issues/90
  • [x] https://github.com/php-tuf/composer-stager/issues/83

Obsolete

  • [x] https://github.com/php-tuf/composer-stager/issues/77 (already a Drupal core dev dependency)

PHPUnit and PHPBench are taken for granted as obvious and indispensable, respectively.

The current list of dev dependencies (that are not already a part of drupal/core, drupal/core-dev, or drupal/coder):

  1. infection/infection
  2. phpbench/phpbench
  3. phpro/grumphp-shim
  4. phpstan/phpstan-strict-rules
  5. rector/rector
  6. symfony/config
  7. thecodingmachine/phpstan-strict-rules

c.f. https://github.com/php-tuf/composer-stager/wiki/Coding-standards-&-style-guide

TravisCarden avatar Mar 01 '23 03:03 TravisCarden

I removed a few dependencies in https://github.com/php-tuf/composer-stager/pull/292.

TravisCarden avatar Oct 25 '23 20:10 TravisCarden

Could we also remove grumphp and phpstan-strict rules and etc.?

The problem is that as soon as one of these dependencies falls behind with PHP 8.4/5 compatibility, it will block composer-stager compatibility with those releases, which in turn could end up blocking Drupal core compatibility with PHP 8.4 - we've had this problem several times with behat/mink and other dev dependencies to the point where we've had to use forks and similar in the past.

catch56 avatar Sep 13 '24 21:09 catch56

The problem is that as soon as one of these dependencies falls behind with PHP 8.4/5 compatibility, it will block composer-stager compatibility with those releases

Is that a meaningful risk since we have full control over the repo and could easily remove any dev dependencies at a moment's notice? Especially since they're all so widely used and responsibly maintained and dev dependencies have no downstream effects.

I consider each remaining dependency to be of high value and low risk as detailed its individual issue above. I humbly request that they be judged individually on their relative merits. If I may be so bold, I think the defect rate on this project has been uncommonly low, and it would be difficult to achieve the same level of quality with the loss of so much of my toolchain. Obviously, I'll yield to the decision of the Drupal core team in the end, but I think it would be a pity to make a judgment decision without it a little deeper consideration.

TravisCarden avatar Sep 16 '24 21:09 TravisCarden

Is that a meaningful risk since we have full control over the repo and could easily remove any dev dependencies at a moment's notice?

If we can commit to removing any dev dependencies as soon as there's a problem (in practice almost always new PHP version compatibility) then I'd be fine with that. Are all the existing dev dependencies already compatible with PHP 8.4?

catch56 avatar Sep 17 '24 09:09 catch56

For me this issue isn't only about PHP compatibility (although that is a critical factor). It's also about the constant stream of dev dependency updates. There have been like 15-18 needed dependency updates just in the past three months.

Currently, @TravisCarden keeps the dependencies up to date for us, and every time I see a dependabot commit go through, I think, "I'm glad I wasn't responsible for that". The thing is, as soon as this becomes a core project as a PM dependency, we are responsible for those updates, and all the other maintenance responsibilities that go with them (PHP compatibility, security updates, conflict resolution, etc.). It's one of the many reasons we try to minimize unnecessary dev dependencies for core.

I understand the value @TravisCarden finds from the extra QA tooling during development, but my recommendation would be to move the extra dependencies and their associated testing suites to a separate project, composer-stager-qa or something, which I think could still run tests for the main project with GitHub actions. That way, we can keep the testing around for as long as we like, but decouple it from core development and maintenance.

xjm avatar Sep 17 '24 16:09 xjm

@catch56 all the existing dev dependencies are currently compatible with PHP 8.4--except for phpspec/prophecy, which of course Drupal has, too.

@xjm your rationale makes sense. I would be open to extracting the extended toolchain to a separate package/repo.

TravisCarden avatar Sep 17 '24 16:09 TravisCarden

https://www.drupal.org/project/drupal/issues/3346707 just landed in core which just made this a bit more urgent.

Ideally we'd be able to release Drupal 11.1 on a stable release of composer-stager, without the additional dev dependencies.

I like the idea of a separate project, is that a pre-requisite to removing the dev dependencies from here or could it be done in parallel?

We also need to move https://www.drupal.org/project/drupal/issues/3331078 on, not exactly sure what the next steps are for that to be honest.

catch56 avatar Oct 25 '24 15:10 catch56

@catch56 I would be fine with removing the dev dependencies and putting them in a separate project in parallel in order to get things unblocked quickly. Shall we proceed that way?

TravisCarden avatar Oct 25 '24 15:10 TravisCarden

Yeah that sounds great thank you!

catch56 avatar Oct 25 '24 15:10 catch56

FYI: I'm starting on this today. If anyone knows of an established pattern (I'm not sure drupal/core-dev correlates exactly) or a clear precedent I should consult, I would be glad to hear about it.

TravisCarden avatar Nov 07 '24 20:11 TravisCarden

I'm not aware of good precedents, agreed core-dev probably isn't a good model.

My imagined 'good' workflow would be that when you have a git clone of this repo, you can composer require the dev repo, and get the stricter tooling pulled in.

And that when you want to work on the dev repo, you could have a git clone of that and composer-require this repo in.

That would require neither being a dependency of the other but the various config files and commands working regardless of which you're working on.

However if it only works one way, then pulling the dev repo in from this one seems like the thing to optimise for.

catch56 avatar Nov 07 '24 21:11 catch56

FYI, in case anyone's watching this: very shortly after picking this issue up I got pulled off to deal with a Symfony Process update that broke our Windows integration. 😒 I'll report back when I return to this issue.

TravisCarden avatar Nov 18 '24 20:11 TravisCarden

I've removed the dependencies in https://github.com/php-tuf/composer-stager/pull/401 so this issue can move forward while I devise a new solution. I'll report back with what I come up with.

TravisCarden avatar Nov 23 '24 00:11 TravisCarden

Okay, the work has been done in https://github.com/php-tuf/composer-stager/pull/404. To summarize it, I created a separate Composer project in its own /tools directory that gets installed separately and doesn't affect the main project's dependency tree or trigger Dependabot PRs. More details in the PR, if you want them. Thanks. 🙂

TravisCarden avatar Dec 12 '24 05:12 TravisCarden

Thanks that's great, I can't think of a better approach, if there is one could always switch over to it later, but will make things a lot easier/quieter here!

catch56 avatar Dec 12 '24 08:12 catch56