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.

Sign up for GitHub

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

Jump to bottom

Fix PHP 8.1 deprecation in ServiceException constructor #5245

Merged
merged 1 commit into from
May 4, 2022
Merged

Fix PHP 8.1 deprecation in ServiceException constructor #5245

merged 1 commit into from
May 4, 2022

Conversation

enumag
Copy link
Contributor

@enumag enumag commented Apr 29, 2022

No description provided.

@enumag enumag requested review from a team as code owners April 29, 2022 12:16
@enumag
Copy link
Contributor Author

enumag commented May 2, 2022

Actually wait... the parent class doesn't accept null code. Shouldn't the default value be 0?

PHP Deprecated:  Exception::__construct(): Passing null to parameter #2 ($code) of type int is deprecated in /var/www/vendor/google/cloud-core/src/Exception/ServiceException.php on line 54

@enumag
Copy link
Contributor Author

enumag commented May 2, 2022

Updated accordingly. It happens for example in \Google\Cloud\Firestore\SnapshotTrait when creating NotFoundException which extends ServiceException.

Please merge this.

@enumag
Copy link
Contributor Author

enumag commented May 3, 2022

@bshaffer Can you please merge this? It's causing a lot of deprecation errors for us.

@bshaffer
Copy link
Contributor

bshaffer commented May 3, 2022 •
edited
Loading

Hi @enumag
What version of PHP are you using to receive this error?

I don't see how this error would be possible, as we aren't even using type hints for that argument. Are you getting this warning from a static analyzer?

@enumag
Copy link
Contributor Author

enumag commented May 3, 2022 •
edited
Loading

PHP 8.1 but yeah it might be from a static analyzer or Symfony. Not completely sure... does it matter?

@enumag
Copy link
Contributor Author

enumag commented May 3, 2022

I just confirmed it's a PHP 8.1 deprecation. See https://3v4l.org/ZftUl

@enumag enumag changed the title Fix ServiceException constructor Fix PHP 8.1 deprecation in ServiceException constructor May 3, 2022
@bshaffer
Copy link
Contributor

bshaffer commented May 3, 2022

I understand now, it's an error because we are passing null to the parent constructor.

@enumag
Copy link
Contributor Author

enumag commented May 4, 2022

Yup, exactly. Can you merge it then please?

@dwsupplee dwsupplee added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 4, 2022
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label May 4, 2022
dwsupplee
dwsupplee approved these changes May 4, 2022
View reviewed changes
Copy link
Contributor

@dwsupplee dwsupplee left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @enumag

@dwsupplee dwsupplee merged commit 1c72476 into googleapis:main May 4, 2022
@release-please release-please bot mentioned this pull request May 4, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwsupplee dwsupplee dwsupplee approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@enumag @bshaffer @dwsupplee @kokoro-team