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

bump minimum version to php 7.4 #2171

Closed
wants to merge 18 commits into from
Closed

Conversation

DjordyKoert
Copy link
Collaborator

@DjordyKoert DjordyKoert commented Jan 2, 2024

Related to #2136.

According to Packagist less than 1% use PHP 7.2 & less than 1% use PHP 7.3. https://packagist.org/packages/nelmio/api-doc-bundle/php-stats#4

Around 20% of installs still happen on PHP 7.4 which is the reason why I still would like to support this version for now.

@kevinpapst
Copy link

What is the release policy for this bundle?
Does bumping the required PHP version need a new major version of the bundle?

@DjordyKoert
Copy link
Collaborator Author

What is the release policy for this bundle? Does bumping the required PHP version need a new major version of the bundle?

This would result in a major version bump

# Conflicts:
#	ModelDescriber/FormModelDescriber.php
#	ModelDescriber/ObjectModelDescriber.php
#	PropertyDescriber/ArrayPropertyDescriber.php
#	PropertyDescriber/BooleanPropertyDescriber.php
#	PropertyDescriber/CompoundPropertyDescriber.php
#	PropertyDescriber/DateTimePropertyDescriber.php
#	PropertyDescriber/FloatPropertyDescriber.php
#	PropertyDescriber/IntegerPropertyDescriber.php
#	PropertyDescriber/ObjectPropertyDescriber.php
#	PropertyDescriber/PropertyDescriberInterface.php
#	PropertyDescriber/StringPropertyDescriber.php
#	Tests/Functional/TestKernel.php
@DjordyKoert DjordyKoert changed the base branch from master to next-5 February 3, 2024 14:27
@DjordyKoert DjordyKoert deleted the branch nelmio:5.x February 3, 2024 14:45
@DjordyKoert DjordyKoert closed this Feb 3, 2024
@DjordyKoert DjordyKoert reopened this Feb 3, 2024
@DjordyKoert DjordyKoert changed the base branch from next-5 to 5.x February 3, 2024 14:46
@DjordyKoert DjordyKoert marked this pull request as ready for review February 3, 2024 16:16
@@ -25,7 +25,8 @@
"symfony/options-resolver": "^5.4|^6.0|^7.0",
"symfony/property-info": "^5.4|^6.0|^7.0",
"symfony/routing": "^5.4|^6.0|^7.0",
"zircote/swagger-php": "^4.2.15",
"zircote/swagger-php": "^4.6.1",
"phpstan/phpdoc-parser": "^1.0",

Choose a reason for hiding this comment

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

Why in require? Is it used somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is related to failing https://github.com/nelmio/NelmioApiDocBundle/actions/runs/7767859362/job/21185134733?pr=2171

Might make more sense to pin this only in dev dependency

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tested it, this does not work because the service gets removed

Choose a reason for hiding this comment

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

I don't get it, are you adding a new dependency for every using project, because on 7.4 some tests are failing? Sounds wrong to me. If you plan a new major version anyway, kick 7.4.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree that this is wrong. Still need to decide about what to do with this. Ideally I would go for PHP 8.1 & Symfony >6

Choose a reason for hiding this comment

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

I'd always vote for moving forward, but hard to decide for a library, if 20% of all installations are still using PHP 7.4.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I had another look at https://packagist.org/packages/nelmio/api-doc-bundle/php-stats#4 and now it report 10% of installs happen on PHP 7.4

I am considering that PHP 8.1 & Symfony >6 isn't that bad of an idea after all. Also together with the fact that we can still keep releasing fixes for v4.* if needed. This would also really help with cleaning up the codebase to new standards and start using phpstan.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Anyways thanks a lot for your feedback @kevinpapst 😄

Choose a reason for hiding this comment

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

Oh yes, there was a steep drop in January. You have my go 😁 but I have no project on 7, so I am a terrible advisor here. IMO these old EOL versions of PHP can still use old releases.

Other libs might have commercial interests and aim for maximum reach. But this is OSS and maintaining a free library should be fun! PHPStan is a huge bonus and if it keeps your spirits up, then it is worth doing so.

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.

3 participants