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

Gracefully handle null return from $this->getConfig('defaults') #84

Merged
merged 2 commits into from
Nov 2, 2015

Conversation

fuhry
Copy link
Contributor

@fuhry fuhry commented Apr 29, 2015

This resolves an "unsupported operand types" error.

Either this or guzzle/command#26 will resolve the issue, I'm leaving it up to the upstream authors to decide which approach is more appropriate since the return type of getConfig() is a design decision.

This resolves an "unsupported operand types" error.
@alexw23
Copy link

alexw23 commented Jun 10, 2015

👍

@alexw23
Copy link

alexw23 commented Jun 19, 2015

When can we merge this?

@peterfox
Copy link
Contributor

peterfox commented Oct 3, 2015

👍

Although I've created a pull request which might be better as returning a array for all missing keys would be misleading as it should really only do this for defaults which can be set at the point of constructing the client

#96

@fuhry
Copy link
Contributor Author

fuhry commented Nov 2, 2015

@mtdowling Can this, #96, or guzzle/command#26 be reviewed and merged please?

mtdowling added a commit that referenced this pull request Nov 2, 2015
Gracefully handle null return from $this->getConfig('defaults')
@mtdowling mtdowling merged commit 1e3118d into guzzle:master Nov 2, 2015
@fuhry
Copy link
Contributor Author

fuhry commented Nov 4, 2015

Thanks @mtdowling. How does your release schedule work? Is it likely that we will see a new point release with this and other recent fixes?

We're getting to the end of our sprint (Friday) and this is one of the last tickets left, so I'm just gauging whether to wait for a point release, or if we should update our composer.json to point to dev-master for the time being. Thanks!

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.

4 participants