foglcz / JSONRpc2

Generic JSON-RPC v2 implementation
Other
19 stars 10 forks source link

PHP 5.6 "Only variable references should be returned by reference" problem solution. #14

Closed theo1024 closed 8 years ago

theo1024 commented 8 years ago

It seems that in PHP 5.6 changed the way the variable reference are returned from methods. This small change repair it. Some details are available on Stack Overflow.

foglcz commented 8 years ago

Nice catch! :)

Commented on the fix, pls update & squash & repush ;)

theo1024 commented 8 years ago

Ok, fixed, it is in the pull request.

foglcz commented 8 years ago

Perfect, could you squash it into one commit pls?

git reset --soft HEAD~5
git commit -m "commit msg"
theo1024 commented 8 years ago

Hm, not sure if this is right. I use git on quite basic level.

foglcz commented 8 years ago

hmmmm, not quite there yet. Try this:

mkdir testdir
cd testdir

git clone git@github.com:https://github.com/hobrasoft-cz/JSONRpc2
cd jsonrpc2

git reset --soft HEAD~7
git commit -m "Bugfix: Added compatibility with PHP 5.6"
git push --force origin master --all

The pull request will be handled by github automagically then

(I'd do it but really want to keep your username in all glorious git history ;) )

theo1024 commented 8 years ago

Ok, I did it as you wrote, but last command cannot be done with --all parameter (git said "error: --all can't be combined with refspecs"), so I did it without it. But still there are changes I didn't. I'm little bit confused of it.

scottchiefbaker commented 8 years ago

This is an awfully big change set for a simple fix. It looks like some other changes got pulled in to this also? I suggest we clean up the pull request before committing, so we don't accidentally pull in changes we don't want.

@theo1024 can you point out exactly what part you changed to make this 5.6 compatible? I use This library every day with PHP 5.6 and I don't have any issues.

scottchiefbaker commented 8 years ago

Oh I see what happened... You haven't done a git pull in a while. There are quite a few changes that have been committed since you last did that (2014). Basically this pull request includes the last two years of changes, plus your fix. If you do a git pull to get the latest code from this repo, then the pull request should be a lot cleaner.

foglcz commented 8 years ago

@theo1024 hmm, sorry about that rebasing then :( Simplest solution might be just to remove your fork, re-fork, apply change (with one commit only!) and reissue PR

theo1024 commented 8 years ago

I agree, it looks that it will be most efficent way to do that with my level of git skills :)

scottchiefbaker commented 8 years ago

@theo1024 give it time. When I first started git/github I hated it because it was so "complex". Now that I understand it, I see how powerful a tool it is. You'll get there.