Skip to content

CLD-379-added-video-options-global-settings#122

Merged
yuval-cloudinary merged 2 commits intocloudinary:masterfrom
asad-rafter:CLD-379-Add-option-to-specify-Video-Player-settings-globally
Mar 9, 2026
Merged

CLD-379-added-video-options-global-settings#122
yuval-cloudinary merged 2 commits intocloudinary:masterfrom
asad-rafter:CLD-379-Add-option-to-specify-Video-Player-settings-globally

Conversation

@asad-rafter
Copy link
Copy Markdown
Contributor

No description provided.

var logger = require('dw/system/Logger').getLogger('int_cloudinary_pd', 'int_cloudinary_pd');
var constants = require('~/cartridge/experience/utils/cloudinaryPDConstants').cloudinaryPDConstants;

var videoPlayeroptions = {};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@asad-rafter please use capital O

var mergedConfigs = {};
var defaults = videoPlayerOptions || {};
var overrides = configurations;
var key;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@asad-rafter vars should go into the innermost blocks, e.g. to the for loops

var overrideVal = normalizeBoolean(overrides[key]);
if (overrideGlobalConfigs === true) {
mergedConfigs[key] = overrideVal;
} else {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@asad-rafter please use "else if"

overrideGlobalConfigs = normalizeBoolean(overrideGlobalConfigs);
var mergedConfigs = {};
var defaults = videoPlayerOptions || {};
var overrides = configurations;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@asad-rafter why do we need this assignment?

@yuval-cloudinary yuval-cloudinary merged commit 6f96813 into cloudinary:master Mar 9, 2026
2 checks passed
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.

2 participants