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

Explicitly mark nullable parameter #287

Merged
merged 1 commit into from
Jul 26, 2024
Merged

Explicitly mark nullable parameter #287

merged 1 commit into from
Jul 26, 2024

Conversation

cedric-anne
Copy link
Contributor

@cedric-anne cedric-anne commented Jul 26, 2024

Some occurences were missed.

I did not pushed the fixes yet, I want to first check if the updated PHPStan configuration will help to detect them. I do not know how to detect them in the CI.

@cedric-anne
Copy link
Contributor Author

@staabm @phil-davis

This PR is ready to be reviewed.

@phil-davis
Copy link
Contributor

phil-davis commented Jul 26, 2024

I do not know how to detect them in the CI.

I have been playing around to try and work out how to make phpstan find all the problems - I haven't succeeded yet.

It doesn't seem to find examples that are not in a class.

@phil-davis phil-davis merged commit ec0a81b into sabre-io:2.2 Jul 26, 2024
9 checks passed
@cedric-anne cedric-anne deleted the patch-1 branch July 26, 2024 13:24
@cedric-anne
Copy link
Contributor Author

I do not know how to detect them in the CI.

I have been playing around to try and work out how to make phpstan find all the problems - I haven't succeeded yet.

It doesn't seem to find examples that are not in a class.

As far as I remember, in our project, we force the PHP version required in the composer.json file in our CI, to be sure that PHPStan executes its check accordingly to the PHP version defined in the matrix.

Something like composer require "php:>=${{ matrix.php-version }}" --ignore-platform-req=php+ --no-install --no-scripts. The + suffix on php+ indicates to ignore the upper bound of the dependencies, see https://getcomposer.org/doc/03-cli.md#install-i

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