-
-
Notifications
You must be signed in to change notification settings - Fork 155
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
issue 324: change parameter name from options to flags #376
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,15 +27,15 @@ | |
* @param bool $assoc When true, returned objects will be converted | ||
* into associative arrays. | ||
* @param int<1, max> $depth User specified recursion depth. | ||
* @param int $options Bitmask of JSON decode options. | ||
* @param int $flags Bitmask of JSON decode options. | ||
* | ||
* @return mixed | ||
* @throws JsonException if the JSON cannot be decoded. | ||
* @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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
{ | ||
$data = \json_decode($json, $assoc, $depth, $options); | ||
$data = \json_decode($json, $assoc, $depth, $flags); | ||
if (JSON_ERROR_NONE !== json_last_error()) { | ||
throw JsonException::createFromPhpError(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm up for making a PR is that is preferable btw