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

3.x: adjust annotations #954

Merged
merged 3 commits into from
Oct 16, 2023
Merged

3.x: adjust annotations #954

merged 3 commits into from
Oct 16, 2023

Conversation

LordSimal
Copy link
Contributor

closes #946

@LordSimal LordSimal added this to the 3.x (CakePHP 5) milestone Oct 16, 2023
@LordSimal LordSimal force-pushed the 3.x-new-annotations branch 2 times, most recently from 564dab6 to 283856a Compare October 16, 2023 07:51
@LordSimal LordSimal force-pushed the 3.x-new-annotations branch from 283856a to d3cf7d6 Compare October 16, 2023 07:53
@LordSimal LordSimal force-pushed the 3.x-new-annotations branch from 39012ad to 24e79f4 Compare October 16, 2023 07:56
@LordSimal
Copy link
Contributor Author

Had to ignore that stan issue because as you can see in https://github.com/cakephp/bake/actions/runs/6530648862/job/17730339754?pr=954#step:7:67 this will get generated with the lowest chronos version due to the fact, that it doesn't extend Datetime in that specific version

@dereuromark dereuromark merged commit 58a180d into 3.x Oct 16, 2023
8 checks passed
@dereuromark dereuromark deleted the 3.x-new-annotations branch October 16, 2023 10:21
Comment on lines +275 to +278
$annotations[] = "@method \Cake\Datasource\ResultSetInterface<\\{$namespace}\\Model\\Entity\\{$entity}>|false saveMany(iterable \$entities, array \$options = [])";
$annotations[] = "@method \Cake\Datasource\ResultSetInterface<\\{$namespace}\\Model\\Entity\\{$entity}> saveManyOrFail(iterable \$entities, array \$options = [])";
$annotations[] = "@method \Cake\Datasource\ResultSetInterface<\\{$namespace}\\Model\\Entity\\{$entity}>|false deleteMany(iterable \$entities, array \$options = [])";
$annotations[] = "@method \Cake\Datasource\ResultSetInterface<\\{$namespace}\\Model\\Entity\\{$entity}> deleteManyOrFail(iterable \$entities, array \$options = [])";
Copy link
Contributor

Choose a reason for hiding this comment

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

That doesn't look right to me. While those methods could return \Cake\Datasource\ResultSetInterface, they generally return what they were fed with in the $entities argument, which would be iterable<\Cake\Datasource\EntityInterface>.

I think the point of additionally having \Cake\Datasource\ResultSetInterface, was to account for the use-case of passing (modified) query results into these methods.

Copy link
Member

@dereuromark dereuromark Oct 16, 2023

Choose a reason for hiding this comment

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

Indeed, we could use iterable<\Cake\Datasource\EntityInterface>|\Cake\Datasource\ResultSetInterface here

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.

RFC: generics for array collections?
3 participants