-
Notifications
You must be signed in to change notification settings - Fork 735
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
range: Update for PHP 8.3 #2801
Conversation
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.
Some remarks. Will probably require a second pass depending on the changes you do.
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.
LGTM once the three remaining remarks are resolved.
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'Ve now had a look at the rendered version. In addition to the new review marks, consider changing the example to:
<?php
echo implode(', ', range(0, 12)), PHP_EOL;
echo implode(', ', range(0, 100, 10)), PHP_EOL;
echo implode(', ', range('a', 'i')), PHP_EOL;
echo implode(', ', range('c', 'a')), PHP_EOL;
echo implode(', ', range('A', 'z')), PHP_EOL;
?>
with the output:
0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12
0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100
a, b, c, d, e, f, g, h, i
c, b, a
A, B, C, D, E, F, G, H, I, J, K, L, M, N, O, P, Q, R, S, T, U, V, W, X, Y, Z, [, \, ], ^, _, `, a, b, c, d, e, f, g, h, i, j, k, l, m, n, o, p, q, r, s, t, u, v, w, x, y, z
The current example lacks the output entirely is is unncessarily verbose with the loops. The new example also showcases non-letter bytes.
TODO Better examples
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
Co-authored-by: Tim Düsterhus <timwolla@googlemail.com>
I've now merged this, it's not getting better when it sits around for longer. |
TODO Better examplesI can't come up with them, and they can always be added later.