cloudinary / cloudinary_npm

Cloudinary NPM for node.js integration
629 stars 323 forks source link

suggest improving vague types #563

Closed thilllon closed 1 year ago

thilllon commented 2 years ago

Feature request for Cloudinary_NPM SDK …(If your feature is for other SDKs, please request them there)

Explain your use case

… (A high-level explanation of why you need this feature)

As a typescript user, i am appreciated that cloudinary provides typescript library so i can save time to checking which values can be used. However, in some APIs, I need to check official docs because there are object types. I think some types can be much clearer.

Describe the problem you’re trying to solve

… (A more technical view of what you’d like to accomplish, and how this feature will help you achieve it)

Do you have a proposed solution?

… (yes, no? Please elaborate if needed)

Based on cloudinary official docs, some object types can be changed to specific types. If it is ok, I can make pr.

jackiecatania commented 2 years ago

@thilllon we'll be happy to review, can you please make a PR?

tamaracloudinary commented 1 year ago

@thilllon we merged the changes in the parameters you asked for (and some more), is that ok or do you have any further parameters you want to include? Thanks -Tamara

thilllon commented 1 year ago

@thilllon we merged the changes in the parameters you asked for (and some more), is that ok or do you have any further parameters you want to include? Thanks -Tamara

Thanks for your works. Actually, the type I mostly wanted to be provided was SignApiOptions, but current changes still look vague to me, e.g., export type SignApiOptions = Record<string, any>; But I read the comments on the code that some types should be decided after discussion with documentation team. So, it is OK for now. Thanks.