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

Allow psr/log ^3.0 #10454

Merged
merged 1 commit into from
Jan 13, 2022
Merged

Allow psr/log ^3.0 #10454

merged 1 commit into from
Jan 13, 2022

Conversation

simPod
Copy link
Contributor

@simPod simPod commented Jan 13, 2022

Same as in #10158

@Seldaek
Copy link
Member

Seldaek commented Jan 13, 2022

To allow psr/log 3 we'd have to add the PHP 8 type hints IIRC?

@simPod
Copy link
Contributor Author

simPod commented Jan 13, 2022

@Seldaek true, I missed that composer/composer implements that interface as well.

Not sure what is the possible upgrade path here, I guess it is not possible to support (v1+v2) and v3 at the same time as method signatures are incompatible?

@stof
Copy link
Contributor

stof commented Jan 13, 2022

you can support v1+v2+v3 at the same time with 2 conditions:

  • your code requires PHP 7.2+ to have the covariance rules
  • your code implements the methods with return types (to support v3) but without argument types (to support v1)

The first condition is not met in the composer 2.2 LTS branch.

@simPod
Copy link
Contributor Author

simPod commented Jan 13, 2022

I've changed target to master

@simPod simPod changed the base branch from 2.2 to main January 13, 2022 11:26
@stof
Copy link
Contributor

stof commented Jan 13, 2022

not having changes in the BaseIO class is wrong. It does not actually make it compatible with v3 (see the second condition I listed in my previous comment)

@simPod
Copy link
Contributor Author

simPod commented Jan 13, 2022

@stof yup I did but the return types are already there 🤔 So since it was compatible till now I assume it still is. Removing array from array $context should not be necessary, or?

@stof
Copy link
Contributor

stof commented Jan 13, 2022

the array argument type is already there in the v1 signature. So it is indeed fine to have it. the without argument types only applies to the new ones being added in psr/log.

the return types are already there

indeed sorry. I looked at the wrong composer branch.

@Seldaek Seldaek added this to the 2.3 milestone Jan 13, 2022
@Seldaek Seldaek merged commit 4a45718 into composer:main Jan 13, 2022
@Seldaek
Copy link
Member

Seldaek commented Jan 13, 2022

Ok let's give this a shot, I forgot it was possible with 7.2 by ignoring the $message type. Thanks!

@simPod simPod deleted the patch-1 branch January 13, 2022 12:07
emahorvat52 pushed a commit to emahorvat52/composer that referenced this pull request Feb 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants