issue 324: change parameter name from options to flags#376
Conversation
|
Hey @MarcinGladkowski and @dbrekelmans , this was a breaking change (both param renamed and function removed), but has been marked as a minor update. For example, it has broken the package https://github.com/jessarcher/laravel-castable-data-transfer-object here. Is there any chance we can rollback this? |
|
Hi @danilopolani. Thanks for the report! This is a though one, because it's hard to support BC for parameter names. This was one of the issues brought up in the internals discussion on the named arguments RFC. Some big open-source projects specifically state that they don't support named arguments as part of their BC promise. While this library doesn't have a written policy on BC, I personally think it's a lot of effort to support this, probably more than it's worth. If you have strong opinions about this, please feel free to continue the discussion here! |
Definitely a tough one, thank you for the explanation 😄 Gonna submit a PR to that project to fix this behaviour, in the meanwhile I've solved by requiring a fixed (old) version of this package. Thank you again! |
| * @link http://www.php.net/manual/en/function.json-decode.php | ||
| */ | ||
| function json_decode(string $json, bool $assoc = false, int $depth = 512, int $options = 0) | ||
| function json_decode(string $json, bool $assoc = false, int $depth = 512, int $flags = 0): mixed |
There was a problem hiding this comment.
Just as a note, this could be considered a breaking change in PHP 8.x because anyone using the named argument will be faced with a breaking change in a bugfix package release.
EDIT: I see this was reported above. I feel pretty strongly about this issue, especially for this package which aims to have parity with PHP functions of the same name. I am 100% for ensuring that this package stays in alignment with PHP current in terms of parameter names. However, I also think there is very little cost to publishing a new minor, or even major, release when parameter names change. If this change had bumped to 2.3.0 instead of 2.2.3, would anything bad have happened? I doubt it.
| * @throws OpensslException | ||
| * | ||
| */ | ||
| function openssl_random_pseudo_bytes(int $length, ?bool &$strong_result = null): string |
There was a problem hiding this comment.
I just ran into this breaking change when upgrading safe. Shouldn't this method have been moved to the deprecated space instead of just being removed? As i randomly ran into an error after doing a minor update, without a deprecation notice prior.
There was a problem hiding this comment.
I'm up for making a PR is that is preferable btw
5 participants
No description provided.