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 stricter type return to Model::getKey #53287

Draft
wants to merge 1 commit into
base: 11.x
Choose a base branch
from

Conversation

treyssatvincent
Copy link
Contributor

Specify that getKey can only returns a int or a string as I don't think it's supported to have something else as primary key.
The aim is to improve type hinting when this method is used.

I didn't used the PHPDoc: it's suggested in https://laravel.com/docs/11.x/contributions#phpdoc to use native types in signature but most the methods on that class uses PHPDoc so I'm not sure about it.

@treyssatvincent treyssatvincent marked this pull request as draft October 24, 2024 07:47
@crynobone
Copy link
Member

crynobone commented Oct 24, 2024

Marking as draft since tests are failing. This probably due to breaking change introduced in this PR.

@treyssatvincent
Copy link
Contributor Author

Marking as draft since tests are failing. This probably due to breaking change introduced in this PR.

Indeed, it has a breaking change because it changes the return type of a method in the QueueableEntity.

I could change only getKey and keep getQueueableId as is to avoid that.

@treyssatvincent
Copy link
Contributor Author

My bad, I didn't realized that it could be null also on pivot.

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.

2 participants