cloudinary / cloudinary_npm

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

Add `set_partial_override` method to the API #668

Open magdakwiecien opened 4 months ago

magdakwiecien commented 4 months ago

Brief Summary of Changes

What Does This PR Address?

Are Tests Included?

Reviewer, Please Note:

magdakwiecien commented 4 months ago

There's a general error in the pipeline run:

> cloudinary@2.2.0 dtslint /home/travis/build/cloudinary/cloudinary_npm
> tools/scripts/ditslint.sh
Error: Errors in typescript@local for external dependencies:
../node_modules/@types/markdown-it/lib/index.d.ts(151,33): error TS2694: Namespace 'LinkifyIt' has no exported member 'LinkifyIt'.
    at /home/travis/build/cloudinary/cloudinary_npm/node_modules/dtslint/bin/index.js:190:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/travis/build/cloudinary/cloudinary_npm/node_modules/dtslint/bin/index.js:5:58)
jackieros commented 4 months ago

LGTM. As long as @mckomo-cl and @const-cloudinary are ok with the naming, I don't have an opinion there.

@mckomo-cl - As per our discussions on the Confluence spec, I wasn't entirely convinced that "partial override" is the ideal name for the feature, but I won't have time to deep dive into it.

I thought it was still in the spec stage. Didn't realize it was already being implemented in an SDK. Have you & @konforti sync'd with @carlevison on the naming of all the methods?