feat: new switch v2 function #20
Conversation
mudit-postman
requested review from
kunagpal,
shubhbhargav,
pooja-postman and
gmann42
index.js
Outdated
| * name: 'Sample' | ||
| * }); | ||
| */ | ||
| switchV2: function (parentFilename, defaultConfig) { |
There was a problem hiding this comment.
Imo the stack approach is better, this we can keep the interface same.
Why didn't we go with that ?
If need be switch and switchV2 can be merged.
https://stackoverflow.com/a/63039252
cc: @kunagpal
There was a problem hiding this comment.
- Let's do all we can to avoid split functionality. As pointed out earlier, this module will be deprecated in a few months anyway.
- If API Observability has confirmed the above is an undigestable risk, let's provide a more descriptive name than
.switchV2. No one adopting such a function will be aware of the original thread that sparked this change, nor will they have the time to sift through the code to try and comprehend howV2differs from the original.
I have no other review comments, merge and release as you see fit 🙂
index.js
Outdated
| @@ -42,7 +42,7 @@ module.exports = { | |||
| } | |||
|
|
|||
| // Get the file name from which config will be loaded. | |||
| var targetFile = require.resolve(path.join(process.cwd(), 'config', 'env', process.env.SAILS_ENV)); | |||
| const targetFile = require.resolve(path.join(process.cwd(), 'config', 'env', process.env.SAILS_ENV)); | |||
There was a problem hiding this comment.
revert ? let's not make unnecessary changes
| // Finally return the target file's config. | ||
| return require(targetFile); | ||
| }, |
There was a problem hiding this comment.
Wasn't this already present ? Why showing up as newly added 😕
There was a problem hiding this comment.
GitHub diff is showing lines 92 and below as existing.
| * will pick the config value from that file. | ||
| * | ||
| * @param defaultConfig this is the default config which is bases on NODE_ENV value | ||
| * @returns {config} |
There was a problem hiding this comment.
For any other reviewers,
We're okay with code repetition because eventually, there's going to be one version only.
No description provided.