Skip to content
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

Consider supporting YAML for hydra configs #165

Open
cjus opened this issue Nov 28, 2017 · 4 comments
Open

Consider supporting YAML for hydra configs #165

cjus opened this issue Nov 28, 2017 · 4 comments

Comments

@cjus
Copy link
Contributor

cjus commented Nov 28, 2017

This might be useful.
https://www.npmjs.com/package/node-yaml

@sjmcdowall
Copy link
Contributor

I see PR#166 in theory handles this Issue .. but nothing on the PR request has happened??

@cjus
Copy link
Contributor Author

cjus commented Feb 28, 2018

The issue I raised privately (@taurenk and I work together) is that of adding another dependency. At one point I was on a crusade to slim Hydra (core) in size. My reason was that I wanted us to be able to claim that Hydra is a light-weight library.

I routinely use cost-of-modules to check on size.

$ npm install -g cost-of-modules
/Users/cjustiniano/.nvm/versions/node/v8.9.4/bin/cost-of-modules -> /Users/cjustiniano/.nvm/versions/node/v8.9.4/lib/node_modules/cost-of-modules/lib/index.js
+ cost-of-modules@1.0.1
added 16 packages in 1.677s

$ cost-of-modules

Making sure dependencies are installed
npm install --production

up to date in 2.044s

Calculating...


┌──────────────┬────────────┬───────┐
│ name         │ children   │ size  │
├──────────────┼────────────┼───────┤
│ bluebird     │ 0          │ 0.59M │
├──────────────┼────────────┼───────┤
│ redis        │ 3          │ 0.26M │
├──────────────┼────────────┼───────┤
│ redis-url    │ 1          │ 0.17M │
├──────────────┼────────────┼───────┤
│ debug        │ 1          │ 0.06M │
├──────────────┼────────────┼───────┤
│ route-parser │ 0          │ 0.05M │
├──────────────┼────────────┼───────┤
│ umf          │ 0          │ 0.03M │
├──────────────┼────────────┼───────┤
│ uuid         │ 0          │ 0.02M │
├──────────────┼────────────┼───────┤
│ 7 modules    │ 4 children │ 1.01M │
└──────────────┴────────────┴───────┘

One way to support this might be to build a HydraPlugin (https://github.com/flywheelsports/hydra/blob/master/plugins.md) that adds yaml to json conversion.

The following are examples of HydraPlugins:

@sjmcdowall
Copy link
Contributor

Personally I don't have a "dog in this hunt" as it were -- I'm a TOML fan :) The reason I bring this up though is because for those looking from the outside -- seeing "old" PR requests apparently "ignored" is not a good sign for an Open Source project. The goal should be to merge or reject PR's ASAP .. and not leave them lingering .. But that's my opinion ..

@cjus
Copy link
Contributor Author

cjus commented Feb 28, 2018

@sjmcdowall thanks for the feedback. I've asked @taurenk to review here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants