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

Method signature changes (setCookie) #4780

Closed
ValiDrv opened this issue Aug 1, 2022 · 3 comments
Closed

Method signature changes (setCookie) #4780

ValiDrv opened this issue Aug 1, 2022 · 3 comments
Labels
in progress Maintainers are working on this

Comments

@ValiDrv
Copy link

ValiDrv commented Aug 1, 2022

Hello

In version 5, the Swoole\Http\Response->setCookie() changed from:
->setCookie(name: $name,value: $value, expires: $expires, path: $path, domain: $domain, secure: $secure, httponly: $httponly);
to
->setCookie(name: $name,value: $value, expire: $expires, path: $path, domain: $domain, secure: $secure, httponly: $httponly);
(notice the expire vs expires argument)

Since in PHP8 you can use named arguments, this is a braking change that should probably be documented, since allot of times methods can be called with ->setCookie(...$cookieArray), and that can introduce hidden bugs.

Also, since swoole/ide-helper and wiki.swoole.com is not being released in the same time as swoole-src tag, this adds to most complains people have about Swoole, "the documentation".

@NathanFreeman NathanFreeman added the in progress Maintainers are working on this label Aug 2, 2022
@NathanFreeman
Copy link
Member

#4830

deminy added a commit to swoole/ide-helper that referenced this issue Sep 12, 2022
reported in swoole/swoole-src#4780
fixed in swoole/swoole-src#4830

Signed-off-by: Demin Yin <deminy@deminy.net>
@ValiDrv
Copy link
Author

ValiDrv commented Nov 9, 2022

Thank you for renaming the parameter. in v 5.0.1
But please make sure to add it to the TAG release notes next time, since this is a breaking change...

@matyhtf
Copy link
Member

matyhtf commented Nov 12, 2022

@ValiDrv The renaming of the parameter name I think is just a typo fix, should not be a breaking change.

This may indeed be incompatible with named parameters. I'll add it to release note later.

@matyhtf matyhtf closed this as completed Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Maintainers are working on this
Projects
None yet
Development

No branches or pull requests

3 participants