Skip to content

Conversation

@jfeingold35
Copy link
Contributor

No description provided.

'webextensions': true // Chrome
};
export function overrideDefaultEnv(overrideEnv: LooseObject): void {
DEFAULT_ENV_VARS = {...DEFAULT_ENV_VARS, ...overrideEnv};
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your innovative approach here.

Also, wanted to point out that Mike had used an Object.assign(sourceObject, destinationObject) call to assign contents of one Json object into another. Using that approach, here we would do something like:
Object.assign(DEFAULT_ENV_VARS, overrideEnv) and DEFAULT_ENV_VARS would get all the new key/values.
However, for type definition, we were using Record<string, any> and had to add @typescript-eslint/no-explicit-any before each call.

Just wanted to lay this out here so that you had another path to consider and choose whichever you liked.

Copy link
Contributor

@rmohan20 rmohan20 left a comment

Choose a reason for hiding this comment

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

Nice work

Copy link
Contributor

@ranekere-sfdc ranekere-sfdc left a comment

Choose a reason for hiding this comment

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

@jfeingold35 - Please change the docs for Run command (--help section) with the new flag and also the help message. https://github.com/forcedotcom/sfdx-scanner/blob/dev/docs/_articles/en/scanner-commands/run.md

// options.baseConfig. Configuration object, extended by all configurations used with this instance.
// You can use this option to define the default settings that will be used if your configuration files don't configure it.
// If they don't already have a baseConfig property, we'll need to instantiate one.
config['baseConfig'] = config['baseConfig'] || {'env': {}};
Copy link
Contributor

Choose a reason for hiding this comment

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

A couple of stylistic nitpicks:

  1. Can we declare constants (module level should be good enough) for "baseConfig" and "env" so we don't have accidental typos while making changes to this code another time?
  2. How about extracting the new portion into a method of its own? That way code readability will be focused.

@jfeingold35 jfeingold35 merged commit 5a0db3f into dev Jul 9, 2020
@ranekere-sfdc ranekere-sfdc changed the title D/w 7791882 b @w-7791882@ -TypeScript scanner throws "X is undefined" errors for elements of mocha/jasmine/chai etc. Sep 29, 2020
@jfeingold35 jfeingold35 deleted the d/W-7791882-b branch November 9, 2020 20:09
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.

5 participants