Skip to content

Conversation

@dmendelowitz
Copy link
Contributor

Summary

Rather than pass the config file path to mcodeApp, the config will now be parsed in the CLI and passed to the app as an object.

New behavior

The client should behave as before

Code changes

  • Renamed configValidator.js to configUtils.js and moved getConfig() from app.js to there
  • Changed the pathToConfig argument of mcodeApp to config and removed config parsing from that file
  • Added getConfig() call to cli.js, the result is then passed into mcodeApp
  • Tests for getConfig() moved from app.test.js to the newly renamed 'configUtils.test.js'

Testing guidance

  • Make sure extraction still works as expected
  • Make sure config parsing still works as expected (errors still thrown for bad paths to config, etc)

@Dtphelan1
Copy link
Contributor

This looks really good!

The only thing I can think of is exporting the getConfig and validateConfig functions in the main export file. I imagine that those will be useful in the other repos.

@mgramigna mgramigna self-assigned this Jul 19, 2021
Copy link
Contributor

@mgramigna mgramigna left a comment

Choose a reason for hiding this comment

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

Looks great

@Dtphelan1 Dtphelan1 merged commit 5b22a2d into develop Jul 19, 2021
@Dtphelan1 Dtphelan1 deleted the config-as-object branch July 19, 2021 20:48
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.

4 participants