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

Updated GetDescription function and ReadEnv. #59

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mashinapetro
Copy link

Hey, Ilya!
Your project is awesome!
I've tried about 10 similar project, but your is the best.
I propose some changes, which very useful in real production - more informative info from GetDescription method and more information when ReadEnv returns errors.
See detailed description and examples below.
Now user can see more userful information then earlier.
In particular, GetDescription function return text with
information with "required" line.
See example below:

Environment variables:
  GREETING string
        Greeting phrase
        Default: "Hello!"
        Required: true
  DB_HOST string
        Database host
        Required: true
  DB_PORT string
        Database port
        Required: true
  DB_USER string
        Database user name
        Required: true
  DB_PASSWORD string
        Database user password
        Required: true
  DB_NAME string
        Database name
        Required: true

Error from ReadEnv contains information not only about
structure's field, but about env variables for this field as well.
field "Host" is required but the value is not provided. Envs names: "SRV_HOST HOST"
Also, all test changed for the last changes.

Now user can see more userful information then earlier.
In particular, GetDescription function return text with
information with "required" line.
See example below:
Environment variables:
  GREETING string
        Greeting phrase
        Default: "Hello!"
        Required: true
  DB_HOST string
        Database host
        Required: true
  DB_PORT string
        Database port
        Required: true
  DB_USER string
        Database user name
        Required: true
  DB_PASSWORD string
        Database user password
        Required: true
  DB_NAME string
        Database name
        Required: true

Error from ReadEnv contains information not only about
structure's field, but about env variables for this field as well.
"field "Host" is required but the value is not provided.
Envs names: "SRV_HOST HOST""

Also, all test changed for the last changes.
@codecov
Copy link

codecov bot commented Oct 19, 2020

Codecov Report

Merging #59 (7b073db) into master (a188ca0) will increase coverage by 0.16%.
The diff coverage is 100.00%.

❗ Current head 7b073db differs from pull request most recent head 5eac4c4. Consider uploading reports for the commit 5eac4c4 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master      #59      +/-   ##
==========================================
+ Coverage   89.79%   89.95%   +0.16%     
==========================================
  Files           1        1              
  Lines         245      249       +4     
==========================================
+ Hits          220      224       +4     
  Misses         17       17              
  Partials        8        8              
Impacted Files Coverage Δ
cleanenv.go 89.95% <100.00%> (+0.16%) ⬆️

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 a188ca0...5eac4c4. Read the comment docs.

@@ -1,4 +1,4 @@
module github.com/ilyakaznacheev/cleanenv
module github.com/mashinapetro/cleanenv
Copy link
Owner

Choose a reason for hiding this comment

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

Please change back to ilyakaznacheev

Comment on lines +316 to +322
envList := ""
for _, env := range meta.envList {
envList = envList + env + " "
}
envList = strings.Trim(envList, " ")
err := fmt.Errorf("field %q is required but the value is not provided. Envs names: %q",
meta.fieldName, envList)
Copy link
Owner

Choose a reason for hiding this comment

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

We really don't need this part, because the value can come from any source - env, yaml, toml, json, etc. The only thing we know is that the value is empty, and there is a message about it.

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.

None yet

2 participants