-
Notifications
You must be signed in to change notification settings - Fork 323
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
base: master
Are you sure you want to change the base?
Conversation
-> Made changes to its README -> Updated the necessary dependencies -> Optimized the code to current standards
There was a problem hiding this 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.
samples/basic/basic.js
Outdated
|
||
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) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.then((image) => { | |
.then((image) => { |
and continue with 2-spaces indentation throughout the file please where you're using promises.
samples/basic/basic.js
Outdated
@@ -1,116 +1,171 @@ | |||
require('dotenv').load(); | |||
require('dotenv').config(); | |||
var fs = require('fs'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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; |
There was a problem hiding this comment.
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'
@cloudinary-pkoniu I have made the changes around const-var inconsistencies, promise handling indentation and using template literals. Please review them. |
@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. |
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?
Reviewer, Please Note: