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

bumped php to 8.0 and phpstan enhancements #143

Closed

Conversation

oleg-andreyev
Copy link

No description provided.

Copy link
Collaborator

@maxhelias maxhelias left a comment

Choose a reason for hiding this comment

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

Thanks for taking this

paths:
- src
- tests
inferPrivatePropertyTypeFromConstructor: true
ignoreErrors:
Copy link
Collaborator

Choose a reason for hiding this comment

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

We must keep this line as long as we support symfony/validator under 6.1

},
"require": {
"php": ">=7.4",
"php": ">=8.0",
"giggsey/libphonenumber-for-php": "^8.0",
"symfony/framework-bundle": "^4.4|^5.3|^6.0",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can also remove any support above 5.4

Copy link
Collaborator

Choose a reason for hiding this comment

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

As well as the related code 👍

Comment on lines 15 to +24
include:
- php: '7.4'
symfony-require: 4.4.*
- php: '8.0'
symfony-require: 5.3.*
- php: '8.0'
symfony-require: 6.0.*
stability: dev
- php: '8.1'
symfony-require: 6.1.*
- php: '8.1'
symfony-require: 6.2.*
stability: dev
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think some refresh is needed here, i propose something like this :

include:
    # Lowest Deps
    - php: 8.0
      symfony-require: 5.4.*
    # LTS with latest stable PHP
    - php: latest
      symfony-require: 5.4.*
    # Active release
    - php: latest
      symfony-require: 6.2.*
    # Development release
    - php: nightly
      symfony-require: 6.3.*@dev
      stability: dev

/*
* This file is part of the Symfony2 PhoneNumberBundle.
* This file is part of the Symfony PhoneNumberBundle.
Copy link
Collaborator

Choose a reason for hiding this comment

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

😄 👍

@@ -1,7 +1,9 @@
<?php

declare(strict_types=1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Changing every file to strict types is not a bad idea by itself, but it is hazardous and requires double-checking every condition... I'm not sure of it since it would require a lot of work to merge this PR 🤔

Copy link
Author

@oleg-andreyev oleg-andreyev Mar 23, 2023

Choose a reason for hiding this comment

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

Since it's php-cs-fixer it can be easily removed . It can be done in another PR but it's must to leverage strict type.

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