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

samples out of date ? #135

Closed
leviwheatcroft opened this issue Feb 18, 2017 · 3 comments · May be fixed by ironhack-course/lab-profile-app#2 or civilizador/TripleTrip#3
Closed

samples out of date ? #135

leviwheatcroft opened this issue Feb 18, 2017 · 3 comments · May be fixed by ironhack-course/lab-profile-app#2 or civilizador/TripleTrip#3
Assignees
Labels

Comments

@leviwheatcroft
Copy link

This sample uses the pattern upload_stream(params, callback) but in your readme it's upload_stream(callback, params).

In addition, the callback does not appear to be passed an error param.

@roeeba roeeba self-assigned this Feb 20, 2017
@roeeba roeeba added bug and removed question labels Feb 20, 2017
@roeeba
Copy link
Collaborator

roeeba commented Feb 20, 2017

Hi @leviwheatcroft thank you for this feedback. We apologise, but it looks like the README file is outdated.
There are currently two versions running in the package, the first one (callback first) is meant mainly for backwards compatibility (to support developers which used our first versions). The 'correct' version is in the v2 namespace (require('cloudinary').v2), as can be seen in the example here:
https://github.com/cloudinary/cloudinary_npm/blob/master/samples/basic/basic.js

I've forwarded this input to our dev team to review and fix. Thanks again.

In regards to the callback's error param - we're aware of this issue and we plan to resolve it as soon as we can. Please see - #131

@roeeba roeeba removed their assignment Feb 20, 2017
@leviwheatcroft
Copy link
Author

Fair enough.. you should definitely add a notice about this in the readme, even if you can't update all the examples in the readme in the short term. I've been playing around with this api a lot in the last few days and I had no idea about the v2 stuff. I just didn't notice the v2 in the samples require statement.

@roeeba
Copy link
Collaborator

roeeba commented Feb 23, 2017

@leviwheatcroft, I fully agree with you. We'll do our best to make it as clear as possible. Thank you for your inputs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants