Skip to content

managing global configuration - #24

Open
temanisparsh wants to merge 2 commits into
pes-alcoding-club:masterfrom
temanisparsh:master
Open

managing global configuration#24
temanisparsh wants to merge 2 commits into
pes-alcoding-club:masterfrom
temanisparsh:master

Conversation

@temanisparsh

@temanisparsh temanisparsh commented Jul 22, 2020

Copy link
Copy Markdown
Contributor

Description of PR

Improve managing global configuration

Relevant Issues
Fixes #8

Checklist

  • Passes unit tests
  • Passes lint tests

Optional notes

In case the user has not created the config file, it throws an error saying:

WARNING: No configurations found in configuration directory:/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/config
WARNING: To disable this warning set SUPPRESS_NO_CONFIG_WARNING in the environment.

If the file is present but is empty, in that case it throws the following error:

/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/node_modules/config/lib/config.js:182
    throw new Error('Configuration property "' + property + '" is not defined');
          ^
Error: Configuration property "app.auth" is not defined
    at Config.get (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/node_modules/config/lib/config.js:182:11)
    at Object.<anonymous> (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/src/middleware/auth.middleware.ts:19:32)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Module.m._compile (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/node_modules/ts-node/src/index.ts:858:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Object.require.extensions.(anonymous function) [as .ts] (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/node_modules/ts-node/src/index.ts:861:12)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/src/routers/auth.router.ts:5:1)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Module.m._compile (/home/sparsh/Desktop/Sparsh/Projects/alcoding/posterior/node_modules/ts-node/src/index.ts:858:23)
    at Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Object

@aniketnk aniketnk left a comment

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.

Awesome! Errors are super informative.
Resolve the comments, then we can merge.

Still looking into why the lint test is failing. @parthvshah Any thoughts?

Comment thread .gitignore Outdated

# Configuration file
src/config.ts
config/default.json

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.

Shouldn't the default.json exist by default?
The default flow should work without any modifications to the repo. And if the user wants to change the default mechanism, we will have to add in another json, and pass that to the config library (through env or any other mechanism)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, will update the PR

@parthvshah

parthvshah commented Jul 23, 2020

Copy link
Copy Markdown
Member

Still looking into why the lint test is failing. @parthvshah Any thoughts?

Files that have been modified in the commit(s):
File:[.gitignore], File_type:[gitignore]
  - WARN! Failed to get filetype for:[.gitignore]!
File:[README.md], File_type:[md]
File:[config/example.json], File_type:[json]
grep: config/example.json: No such file or directory
grep: config/example.json: No such file or directory
grep: config/example.json: No such file or directory
/action/lib/linter.sh: line 537: config/example.json: No such file or directory
/action/lib/linter.sh: line 556: config/example.json: No such file or directory

So I think this is the issue. Looks like the linter fails if you are trying to add a new file and it isn't present in the repo. This would explain the issue with PR #23 too.

Still looking into this.

@temanisparsh
temanisparsh requested a review from aniketnk July 24, 2020 17:07
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.

Improve managing global configuration

3 participants