Skip to content

Add ability to do a deep lookup into a JSON tree#1

Open
NathanAhlstrom wants to merge 5 commits intokushalpandya:masterfrom
NathanAhlstrom:master
Open

Add ability to do a deep lookup into a JSON tree#1
NathanAhlstrom wants to merge 5 commits intokushalpandya:masterfrom
NathanAhlstrom:master

Conversation

@NathanAhlstrom
Copy link
Copy Markdown

needed for our environment the ability to do a deep lookup into a JSON object, so I've added some bits from lookup.js into this node application. Thanks for building this tool.

Copy link
Copy Markdown
Owner

@kushalpandya kushalpandya left a comment

Choose a reason for hiding this comment

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

Thanks for working on this @NathanAhlstrom, I forgot that I had this project lying around and that it could be cleaned up/improved further. 😄

For now, I've suggested some changes, while you update your PR, I'll also check out the branch locally as this app is due for some major revision. 🙂

* execute permission on the command line scripts.
*/
fnValidateConfig = function(sH) {
for (var i = 0; i < sH.length; i++) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

I know this file for the most part is ES5 code, but we can change this loop to sH.forEach and then also use template strings within _.lookup for cleaner string concatenation. What do you think? 🙂

return false;
}

if ( token === secretKey ) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We can just do return token === secretKey; here and get rid of entire condition block.

fs.constants.F_OK | fs.constants.R_OK |
fs.constants.X_OK);
} catch (e) {
console.log("Missing file or execute permissions on file: " + file);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Using console.error would be appropriate here.

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