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

Fixes issue #896 #897

Closed
wants to merge 1 commit into from
Closed

Conversation

justin-schroeder
Copy link

In this PR:

  • Fixes issue introduced in PHP 7.4 with accessing an integer as an array (123[0] now throws a notice)

@stripe-ci
Copy link

A pull request should not be used to merge the main branch (master)
into a feature branch (master). Reversed pull requests can trigger
performance issues in CIBot and GitHub Enterprise. If you accidentally reversed the
base and merge branches when creating the pull request, please create a new pull request.
Ask in #dev-productivity if you have any questions!

@stripe-ci stripe-ci closed this Mar 18, 2020
@ob-stripe
Copy link
Contributor

Thanks for the fix @justin-schroeder. FWIW I think the @stripe-ci bot is a bit too aggressive here: it wanted you to create the PR from a feature branch in your fork rather than from the master branch. I don't think that's necessary for PRs that come from external forks, this should probably only apply to our internal repos anyway.

I've opened a PR based on your fix here: #899. I added a test to make sure we're covering the bug, and explicitly converted the key to string to comply with PHP 7.1's strict types (we're not yet using them because we're still supporting older PHP versions at this time, but we will eventually).

@justin-schroeder
Copy link
Author

Great thanks @ob-stripe. Yeah I couldn't quite figure out what the bot was asking me to do, and decided to bail so there weren't 15 closed PRs ha.

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.

3 participants