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

CurlFile fix #53

Merged
merged 3 commits into from
Nov 15, 2018
Merged

CurlFile fix #53

merged 3 commits into from
Nov 15, 2018

Conversation

nadar
Copy link
Member

@nadar nadar commented Nov 14, 2018

We are facing the same problem with CurlFile Uploads (#46).

Please take a closer look at the fix in order to ensure its backwards compatible.

This is not tested, but we are facing the same problem with CurlFile Uploads (php-mod#46) - This *should* do the trick.
@nadar
Copy link
Member Author

nadar commented Nov 15, 2018

Maybe a check if data is an array should be done, otherwise the foreach would throw an exception if its an object - but what could be situation with an object?

@amouhzi
Copy link
Member

amouhzi commented Nov 15, 2018

Thank you for your pull request.

I updated the master branch to fix tests. Can you please update your pull request with recent changes ?

@nadar
Copy link
Member Author

nadar commented Nov 15, 2018

@amouhzi should be done, please take a look.

Maybe you can trigger the build again: https://travis-ci.org/php-mod/curl/builds/455089991

@nadar
Copy link
Member Author

nadar commented Nov 15, 2018

(just saw you where switching to gitlab ci, well then the build badge can be removed)

@amouhzi amouhzi changed the base branch from master to nadarissue-46 November 15, 2018 21:35
@amouhzi amouhzi merged commit ac02d84 into php-mod:nadarissue-46 Nov 15, 2018
@amouhzi
Copy link
Member

amouhzi commented Nov 15, 2018

It doesn't pass CS tests https://gitlab.com/anezi/curl/-/jobs/121211929

Something has changed in Github, Travis or Gitlab. The tests did not turn before merging the branch.

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.

2 participants