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

Updated Sample Projects #698

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

aarya-16
Copy link

Brief Summary of Changes

Updated sample projects by doing the following:

→ Updated any outdated dependencies and replaced any deprecated dependencies
→ Refactored code to follow ES6 standards
→ Added some new use cases
→ Changed and improved documentation and comments wherever necessary.

What Does This PR Address?

Are Tests Included?

  • Yes
  • No

Reviewer, Please Note:

-> Made changes to its README
-> Updated the necessary dependencies
-> Optimized the code to current standards
Copy link
Contributor

@cloudinary-pkoniu cloudinary-pkoniu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall the changes look ok, thank you for your work :)
Take a look at the comments that I left, only related to cosmetics.


console.log("** ** ** ** ** ** ** ** ** Uploads ** ** ** ** ** ** ** ** ** **");

// File upload
cloudinary.uploader.upload('pizza.jpg', { tags: 'basic_sample' }, function (err, image) {
cloudinary.uploader.upload('pizza.jpg', { tags: 'basic_sample' })
.then((image) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.then((image) => {
.then((image) => {

and continue with 2-spaces indentation throughout the file please where you're using promises.

@@ -1,116 +1,171 @@
require('dotenv').load();
require('dotenv').config();
var fs = require('fs');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var fs = require('fs');
const fs = require('fs');

I see that in other files var's were replaced with const's, so please include these changes here as well.

}

function add_direct(req, res) {
// Configuring cloudinary_cors direct upload to support old IE versions
var cloudinary_cors = "http://" + req.headers.host + "/cloudinary_cors.html";
const cloudinary_cors = "http://" + req.headers.host + "/cloudinary_cors.html";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const cloudinary_cors = "http://" + req.headers.host + "/cloudinary_cors.html";
const cloudinary_cors = `http://${req.headers.host}/cloudinary_cors.html`;

Use template literals.

// via online console or API not related to the actual upload
var sha1 = crypto.createHash('sha1');
// In 'real life' scenario the preset name will be meaningful and will be set via online console or API not related to the actual upload
let sha1 = crypto.createHash('sha1');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
let sha1 = crypto.createHash('sha1');
const sha1 = crypto.createHash('sha1');

Same story as with var. This can be a const variable.

@@ -22,7 +22,199 @@
NOT CONTROL.

*/
var JSON;if(!JSON){JSON={}}(function(){function str(a,b){var c,d,e,f,g=gap,h,i=b[a];if(i&&typeof i==="object"&&typeof i.toJSON==="function"){i=i.toJSON(a)}if(typeof rep==="function"){i=rep.call(b,a,i)}switch(typeof i){case"string":return quote(i);case"number":return isFinite(i)?String(i):"null";case"boolean":case"null":return String(i);case"object":if(!i){return"null"}gap+=indent;h=[];if(Object.prototype.toString.apply(i)==="[object Array]"){f=i.length;for(c=0;c<f;c+=1){h[c]=str(c,i)||"null"}e=h.length===0?"[]":gap?"[\n"+gap+h.join(",\n"+gap)+"\n"+g+"]":"["+h.join(",")+"]";gap=g;return e}if(rep&&typeof rep==="object"){f=rep.length;for(c=0;c<f;c+=1){if(typeof rep[c]==="string"){d=rep[c];e=str(d,i);if(e){h.push(quote(d)+(gap?": ":":")+e)}}}}else{for(d in i){if(Object.prototype.hasOwnProperty.call(i,d)){e=str(d,i);if(e){h.push(quote(d)+(gap?": ":":")+e)}}}}e=h.length===0?"{}":gap?"{\n"+gap+h.join(",\n"+gap)+"\n"+g+"}":"{"+h.join(",")+"}";gap=g;return e}}function quote(a){escapable.lastIndex=0;return escapable.test(a)?'"'+a.replace(escapable,function(a){var b=meta[a];return typeof b==="string"?b:"\\u"+("0000"+a.charCodeAt(0).toString(16)).slice(-4)})+'"':'"'+a+'"'}function f(a){return a<10?"0"+a:a}"use strict";if(typeof Date.prototype.toJSON!=="function"){Date.prototype.toJSON=function(a){return isFinite(this.valueOf())?this.getUTCFullYear()+"-"+f(this.getUTCMonth()+1)+"-"+f(this.getUTCDate())+"T"+f(this.getUTCHours())+":"+f(this.getUTCMinutes())+":"+f(this.getUTCSeconds())+"Z":null};String.prototype.toJSON=Number.prototype.toJSON=Boolean.prototype.toJSON=function(a){return this.valueOf()}}var cx=/[\u0000\u00ad\u0600-\u0604\u070f\u17b4\u17b5\u200c-\u200f\u2028-\u202f\u2060-\u206f\ufeff\ufff0-\uffff]/g,escapable=/[\\\"\x00-\x1f\x7f-\x9f\u00ad\u0600-\u0604\u070f\u17b4\u17b5\u200c-\u200f\u2028-\u202f\u2060-\u206f\ufeff\ufff0-\uffff]/g,gap,indent,meta={"\b":"\\b","\t":"\\t","\n":"\\n","\f":"\\f","\r":"\\r",'"':'\\"',"\\":"\\\\"},rep;if(typeof JSON.stringify!=="function"){JSON.stringify=function(a,b,c){var d;gap="";indent="";if(typeof c==="number"){for(d=0;d<c;d+=1){indent+=" "}}else if(typeof c==="string"){indent=c}rep=b;if(b&&typeof b!=="function"&&(typeof b!=="object"||typeof b.length!=="number")){throw new Error("JSON.stringify")}return str("",{"":a})}}if(typeof JSON.parse!=="function"){JSON.parse=function(text,reviver){function walk(a,b){var c,d,e=a[b];if(e&&typeof e==="object"){for(c in e){if(Object.prototype.hasOwnProperty.call(e,c)){d=walk(e,c);if(d!==undefined){e[c]=d}else{delete e[c]}}}}return reviver.call(a,b,e)}var j;text=String(text);cx.lastIndex=0;if(cx.test(text)){text=text.replace(cx,function(a){return"\\u"+("0000"+a.charCodeAt(0).toString(16)).slice(-4)})}if(/^[\],:{}\s]*$/.test(text.replace(/\\(?:["\\\/bfnrt]|u[0-9a-fA-F]{4})/g,"@").replace(/"[^"\\\n\r]*"|true|false|null|-?\d+(?:\.\d*)?(?:[eE][+\-]?\d+)?/g,"]").replace(/(?:^|:|,)(?:\s*\[)+/g,""))){j=eval("("+text+")");return typeof reviver==="function"?walk({"":j},""):j}throw new SyntaxError("JSON.parse")}}})();
var JSON;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this was minified code, right? Can we keep it this way?

-> Used template literals
-> Improved indentations around promise handling
-> Switched from 'var' to 'const'
@aarya-16
Copy link
Author

aarya-16 commented Oct 29, 2024

@cloudinary-pkoniu I have made the changes around const-var inconsistencies, promise handling indentation and using template literals. Please review them.
And about the minified code, I made that changes because they made it look better, and plus that is the only code in the file.

@const-cloudinary
Copy link

@aarya-16 thanks for your contribution. This PR is eligible for free Cloudinary Hackatoberfest swag. Please send me an email at [email protected] with your name, GitHub username, and a link to the PR where I'll send further instructions.

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.

3 participants