-
Notifications
You must be signed in to change notification settings - Fork 10
added feature to use json files in cli #7 4d #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Several changes needed.
- Please follow code syntax patterns in each file you are working on. Try not to introduce new patterns, unless it's absolutely necessary. In this case your comments are wildly and inconsistently formatted.
?
is an escape pattern that may cause additional bugs down the line. In this particular case, since the tests were passing, these additions to the code were unnecessary.
} | ||
//parse schema | ||
const schema = parse(fs.readFileSync(this.input, 'utf8')); | ||
// read input file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
follow commenting pattern in file. no spaces after //
ie. //read input file
const schema = parse(fs.readFileSync(this.input, 'utf8')); | ||
// read input file | ||
const content = fs.readFileSync(this.input, 'utf8'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this empty line.
// read input file | ||
const content = fs.readFileSync(this.input, 'utf8'); | ||
|
||
// parse schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
follow commenting pattern in file. no spaces after //
ie. //parse schema
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
follow commenting pattern in file. Avoid lateral commenting. Commenting should be above the code in question. ie.
//parse directly
? JSON.parse(content)
const schema: SchemaConfig = path.extname(this.input) === '.json' | ||
? JSON.parse(content) // parse directly | ||
: parse(content); // parse as normal | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove this empty line.
} | ||
} | ||
if (child.model) { | ||
if (child?.model) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove ?
. Tests still pass without this.
schema.model = schema.model || {}; | ||
//loop through child types | ||
for (const [ name, model ] of Object.entries(child.model)) { | ||
for (const [ name, model ] of Object.entries(child?.model)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove ?
. Tests still pass without this.
//ensure the plugin not exists or is not object | ||
//if the conditions are true will throw an error. | ||
if (!this.schema.plugin || typeof this.schema.plugin !== 'object') { | ||
if (!this.schema?.plugin || typeof this.schema?.plugin !== 'object') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove ?
. Tests still pass without this.
} | ||
//loop through plugins | ||
for (const plugin in this.schema.plugin) { | ||
for (const plugin in this.schema?.plugin) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove ?
. Tests still pass without this.
const module = this.loader.absolute(plugin); | ||
//get the plugin config | ||
const config = this.schema.plugin[plugin] as Record<string, any>; | ||
const config = this.schema?.plugin[plugin] as Record<string, any>; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove ?
. Tests still pass without this.
What is this PR for?
I verify that...
npm run test
with no errors