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

Add wildcard support to ignore platform req #10083

Conversation

herndlm
Copy link
Contributor

@herndlm herndlm commented Aug 29, 2021

This is the implementation of what is described in #10045 and the follow-up of #10079.

@herndlm
Copy link
Contributor Author

herndlm commented Aug 29, 2021

As #10079 is not merged yet this PR contains that PR's commit too. After it got merged this one needs to be re-targeted.

@Seldaek Seldaek added this to the 2.2 milestone Sep 2, 2021
@Seldaek Seldaek added the Feature label Sep 2, 2021
@herndlm herndlm requested a review from Seldaek September 2, 2021 18:39
@herndlm herndlm requested a review from Seldaek September 4, 2021 10:41
@herndlm herndlm requested a review from Seldaek September 5, 2021 11:20
Copy link
Member

@Seldaek Seldaek left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM now, will need a rebase once the other PR is merged but I can do that myself when branching out 2.2 if needed :)

@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from 9a51544 to 943db45 Compare September 5, 2021 11:45
@herndlm
Copy link
Contributor Author

herndlm commented Sep 5, 2021

Great, I could not resist cleaning up the fixup commits, I'll leave the rest to you now, except you need me to resolve merge conflicts or so :)

Btw unrelated, but if you need help in increasing the phpstan level or cleanup some more code for phpstan by adding/fixing types and so on, feel free to create issues for that or ping me. I would gladly help out, but will not touch too much if you're also working on it

@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from 943db45 to 70e337a Compare September 5, 2021 12:10
@herndlm
Copy link
Contributor Author

herndlm commented Sep 5, 2021

Also: I had to remove the static keyword in the anonymous function in array_map. PHP 5.3 was not quite happy with that..

@Seldaek
Copy link
Member

Seldaek commented Sep 5, 2021

Btw unrelated, but if you need help in increasing the phpstan level or cleanup some more code for phpstan by adding/fixing types and so on, feel free to create issues for that or ping me. I would gladly help out, but will not touch too much if you're also working on it

Yeah that would be great, I have been working on typing all properties already, because that gives a good foundation to then type method return values and parameters, which will then force more typing in various places to fix usage warnings surely.

I'm almost done with properties, then maybe I can create an issue with some instructions how to run this and what would be the most valuable, but I'm not entirely sure how much is worth doing right now and how much would be best to wait for us to drop old PHP then we can add proper scalar types instead of adding docblocks now and having to migrate them in a few months. So yeah happy to get help at some point but probably it's too soon :)

@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from 70e337a to 7a2cf5f Compare September 5, 2021 17:17
@herndlm
Copy link
Contributor Author

herndlm commented Sep 5, 2021

rebased to fix a conflict

@staabm
Copy link
Contributor

staabm commented Sep 11, 2021

types instead of adding docblocks now and having to migrate them in a few months

We can automate this step with rector

@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from 7a2cf5f to d2ad193 Compare September 29, 2021 15:05
@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from d2ad193 to 439a974 Compare October 13, 2021 08:03
@herndlm
Copy link
Contributor Author

herndlm commented Oct 13, 2021

rebased to fix a conflict

@Seldaek
Copy link
Member

Seldaek commented Oct 14, 2021

As promised in #10083 (comment) - here is #10159 if you still want to help @herndlm (or anyone:)

@herndlm herndlm force-pushed the feature/add-wildcard-support-to-ignore-platform-req branch from 439a974 to 0e93ff9 Compare November 11, 2021 12:19
@herndlm
Copy link
Contributor Author

herndlm commented Nov 11, 2021

rebased after the precondition was merged 🎉

@herndlm herndlm requested a review from Seldaek November 11, 2021 12:24
@Seldaek Seldaek merged commit 7eca450 into composer:main Nov 11, 2021
@Seldaek
Copy link
Member

Seldaek commented Nov 11, 2021

Thanks!

@herndlm herndlm deleted the feature/add-wildcard-support-to-ignore-platform-req branch November 11, 2021 15:10
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