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

[config] Deprecated config (v1) package #80

Merged
merged 3 commits into from
Aug 8, 2018
Merged

Conversation

at15
Copy link
Member

@at15 at15 commented Aug 8, 2018

  • config package was created to use template, however, for normal application, unmarshal YAML to struct is already enough, the only func I use from config package is LoadYAMLAsStruct
  • cast package is too small to be a top level package
  • bumped version in Makefile, it was 0.0.1 all the way ....

About the future direction of config package

  • use reflect/generator struct to accept override using environment variables, i.e. APP_HTTP_PORT, APP_SERVICE_ENABLED etc.
  • validation logic
  • show concrete output, this require new parser like the never going to finish https://github.com/at15/yayaml

As for config

  • might remove using yaml as an intermediate to convert interface to struct, after all json is in stdlib and YAML is not

- cast was based on https://github.com/spf13/cast but it only
implemented a small amount of methods because in config we no longer use
string path to find value in a complex untyped struct, and unmarshal it
to struct directly
- the original aim of config package is to support template inside YAML,
kind of like what Ansible is doing, it was invented for Xephon-B.
We first use a non standard template engine (I even forgot its name
now), and then switched to using standard library's `text/template`,
this syntax is a bit awkward when for loop is used, it's ok when using
things like `{{env KEY_NAME}}`
- however, when not writing bechmark config, the template feature is not
that useful, normally I just unmarshal the yaml to struct and that's it
- I am not sure what v2 of config would be, maybe support more format or
enforce the struct/interface of config struct. (It used to have a
validation interface that check the config and its fields manually)
or just allow downloading config file from the wire
- move cast package to util/cast
- deprecated config package, move to legacy/config
@codecov
Copy link

codecov bot commented Aug 8, 2018

Codecov Report

Merging #80 into master will decrease coverage by 2.32%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
- Coverage   43.28%   40.96%   -2.33%     
==========================================
  Files          53       51       -2     
  Lines        1915     1738     -177     
==========================================
- Hits          829      712     -117     
+ Misses       1015      972      -43     
+ Partials       71       54      -17
Impacted Files Coverage Δ
util/cast/cast.go 61.9% <ø> (ø)
generator/generate.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f0644e8...0066ca3. Read the comment docs.

@at15 at15 merged commit 1eed5cf into master Aug 8, 2018
@at15 at15 deleted the config/v1/deprecated branch August 8, 2018 03:32
at15 pushed a commit to dyweb/go.ice that referenced this pull request Aug 31, 2018
at15 added a commit to dyweb/go.ice that referenced this pull request Dec 9, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant