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

Register mandatory and non-mandatory env vars #22

Merged
merged 2 commits into from
Oct 2, 2018

Conversation

lucasgomide
Copy link
Contributor

@lucasgomide lucasgomide commented Oct 2, 2018

This commit closes issue #21


This change is Reviewable

@lucasgomide lucasgomide force-pushed the log-env branch 2 times, most recently from 3aaf3e0 to 6512a16 Compare October 2, 2018 17:59
Copy link
Owner

@danielfireman danielfireman left a comment

Choose a reason for hiding this comment

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

Super cool stuff!

Only small changes below. Furthermore, I believe you need to run dep ensure to update Gopkg files as you introduced envconfig.

// Specification represents a map of enviornment variables
type Specification struct {
UserPassword string `envconfig:"USERPASSWD" required:"true"`
EncryptionKey string `split_words:"true" required:"true"`
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: Better to use envconfig:"ENCRYPTION_KEY" than split_words (which ends up doing the same thing, but indirectly).

Same bellow.

}
key := []byte(os.Getenv("ENCRYPTION_KEY"))

Copy link
Owner

Choose a reason for hiding this comment

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

Log the environment variables/config?

fmt.Printf("%+v", spec) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I forgot to log it.. dammit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@lucasgomide
Copy link
Contributor Author

@danielfireman done (:

@danielfireman danielfireman merged commit 9415880 into danielfireman:master Oct 2, 2018
@danielfireman
Copy link
Owner

Thanks a lot, @lucasgomide ! :D

@lucasgomide lucasgomide deleted the log-env branch October 2, 2018 18:39
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.

Environment variable mandatory
2 participants