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

Fix composer partial load #45

Closed

Conversation

kainxspirits
Copy link
Contributor

The actual version of this package does not work with the next version of composer/composer.

Running the unit tests will return this output :

TypeError: Ramsey\CaptainHook\PrepareConventionalCommit::getComposer(): Return value must be of type Composer\Composer, Composer\PartialComposer returned

Description

The fullyLoaded attribute was explicitly set to false, so I changed it to true in order to get a Composer\Composer instance instead of a Composer\PartialComposer one.

You can also refer to this pull request to get more context about this change in composer : composer/composer#10547.

It won't introduce any breaking changes (the fullyLoaded argument already existed in composer:^1.10) and it should be compatible with all the php versions supported by this package.

You can find all the internal changes for composer 2.3 here : composer/composer#10573.

I also updated the totallyTyped attribute from the Psalm config since it's deprecated and was raising an error : https://psalm.dev/docs/running_psalm/configuration/#totallytyped. I'm not familiar with Psalm but it should have the same behaviour as before.

Motivation and context

We need this change to make this package works with the next version of composer/composer (version 2.3.0-RC2 at the moment).

This package just doesn't work for those who are already using the 2.3.0.

How has this been tested?

I didn't add any new tests as the ones already present cover this change : https://github.com/ramsey/conventional-commits/blob/1.3.0/tests/ConventionalCommits/Configuration/FinderToolTest.php#L185.

I also made some some manual tests locally (php8.0 and php8.1) and it works fine.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

PR checklist

  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING.md document.
  • I have added tests to cover my changes.

Copy link
Owner

@ramsey ramsey left a comment

Choose a reason for hiding this comment

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

Thanks!

@ramsey
Copy link
Owner

ramsey commented Mar 31, 2022

The tests are failing because of some issues with my devtools lib, which I need to fix. 😕

@ollieread
Copy link

Hey @ramsey, anything I can do to help you get this ready for merge?

@ramsey
Copy link
Owner

ramsey commented Apr 5, 2022

I'll need to separate out the tools from ramsey/devtools and use them directly. My experiment with building a single package to manage all my development tools hasn't worked out so well, since Composer uses an older version of symfony/console internally and plugins end up using the classes bundled with Composer rather than the classes installed to a project's vendor/ directory. 😦

@ramsey
Copy link
Owner

ramsey commented Apr 20, 2022

I've cherry-picked this into #48 and will merge it soon. Thanks!

@ramsey ramsey closed this Apr 20, 2022
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.

None yet

3 participants