Skip to content

Array and string offset access syntax with curly braces is deprecated#17

Merged
electrolinux merged 2 commits intoelectrolinux:masterfrom
n1c:master
Oct 9, 2021
Merged

Array and string offset access syntax with curly braces is deprecated#17
electrolinux merged 2 commits intoelectrolinux:masterfrom
n1c:master

Conversation

@n1c
Copy link
Copy Markdown

@n1c n1c commented Dec 18, 2019

Not sure if there are more examples of it; but this triggered in my app! The change makes it supported by PHP 7.4.

@OneCodeMonkey
Copy link
Copy Markdown

This repo seems nobody maintains, php version is too old.
And because of many static variables usage, it has an memory leak risk.

@electrolinux electrolinux merged commit cfe59d2 into electrolinux:master Oct 9, 2021
@caramboleyo
Copy link
Copy Markdown

caramboleyo commented Nov 20, 2021

@OneCodeMonkey

This repo seems nobody maintains, php version is too old. And because of many static variables usage, it has an memory leak risk.

Why is the use of static variables a memory leak risk?

@valerio-bozzolan
Copy link
Copy Markdown

Just as a note, this fix was resolved also in this fork (that seems more updated):

https://github.com/rubobaquero/phpquery

Comment thread composer.json
@@ -1,5 +1,5 @@
{
"name": "electrolinux/phpquery"
"name": "n1c/phpquery"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wonder if this one should've changed...

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, in fact.
At first, it doesn't bother me as I was no longer using (nor really maintaining) this package, but as it is still the first one shown when requesting phpquery at packagist.org, it seems @n1c should not have changed this unless also taking care of the packagist repo...

@reedy, do you have a clue as to how to make some advertising in package.json to tell people requiring it that this package need a new maintainer ?
Are you interested ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you visit https://packagist.org/packages/electrolinux/phpquery when logged in as the maintainer/owner of the package, you can hit abandon which will make it fairly obvious. You can un-abandon it later...

It does seem some forks like https://packagist.org/packages/rubobaquero/phpquery are more actively maintained though

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah this is definitely bad. Sorry about the confusion caused!

I think it was because I was using my own fork under that namespace, but it shouldn't have gone into the PR to get merged back into this repo.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't worry, I'm the one that merged this PR! :)
And we often learn more from our mistakes than from our successes...

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big deal when there's been no release with it.. yet ;)

#19 reverts the offending commit.

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.

6 participants