-
Notifications
You must be signed in to change notification settings - Fork 0
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
New API #14
Conversation
serum.go
Outdated
@@ -16,17 +16,30 @@ type Injector struct { | |||
envVars *envparser.EnvVars | |||
} | |||
|
|||
// NewInjector creates a new injector using the provided options. | |||
func NewInjector(options ...Option) (*Injector, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we agreed to have the from
as a required param NewInjector(from fInterface, options ...Option)
. otherwise the API is bit confusing and error prone
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I must have misunderstood. That works too. I currently have them as Options. I could make a separate type for the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated! The first param now is an interface called a Loader. It's responsible for loading env vars from an arbitrary source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great job Rob!
Beside the defer ij.SecretProvider.Close()
I think we have a great api now.
Yea I don't like that either. We can move into the inject but then the user loses control. Either way, we can move this later. |
sounds good 🙌 |
This majorly overhauls Serum's API.
It allows you to provide options to initialize serum in a declarative way.
It adds a new source
FromEnv
which allows Serum to read existing environment variables and decrypt any if necessary.