-
Notifications
You must be signed in to change notification settings - Fork 1
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
[WIP] Read env variables from file #5
base: master
Are you sure you want to change the base?
[WIP] Read env variables from file #5
Conversation
Codecov Report
@@ Coverage Diff @@
## master #5 +/- ##
==========================================
- Coverage 57.94% 55.92% -2.03%
==========================================
Files 8 8
Lines 302 304 +2
==========================================
- Hits 175 170 -5
- Misses 106 111 +5
- Partials 21 23 +2
Continue to review full report at Codecov.
|
env.go
Outdated
return err | ||
} | ||
if err == io.EOF && len(pair) == 0 { | ||
return nil |
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.
if err != nil {
if err == io.EOF && len(pair) == 0 {
return nil
}
return err
}
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.
Here you made error (proebalis`)
env.go
Outdated
ErrInvalidPair = errors.New("invalid pair for env variable") | ||
) | ||
|
||
type EnvConfig struct{} |
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.
please add comment
env.go
Outdated
return &EnvConfig{} | ||
} | ||
|
||
func (e *EnvConfig) SetEnv(data io.Reader) 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.
should be separated logic for parsing env file and setting env variables
env.go
Outdated
ErrInvalidPair = errors.New("invalid pair for env variable") | ||
) | ||
|
||
type EnvConfig struct{} |
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.
also would be nice if this struct implements external interface
env.go
Outdated
pair = bytes.Trim(pair, "\n") | ||
i := bytes.Index(pair, []byte("=")) |
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.
read how to properly parse env file
env.go
Outdated
) | ||
|
||
var ( | ||
ErrInvalidPair = errors.New("invalid pair for env variable") |
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.
comment
env.go
Outdated
return nil | ||
} | ||
} | ||
n = bytes.Trim(n, " ") |
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.
simply use strings.TrimSpace
env.go
Outdated
) | ||
|
||
const ( | ||
commentSymbol = '#' |
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.
can be defined in Parse func scope
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.
@antonmashko I think this comment relates more to the style of the code.
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.
yep, it is
env.go
Outdated
if i == -1 { | ||
return ErrInvalidPair | ||
} | ||
key := string(n[:i]) |
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.
- key and value also should be trimmed on white spaces
- strings.Trim(value, "`"")
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.
- We must enable the user to specify a space at the beginning/end of the line/key;
- ``` this is invalid chapter for
env
variables.
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.
- What do you mean? If you try in bash define variable like
FOO=" BAR"
and printFOO
variable, result will beBAR
without spaces. - Sorry, my fault. I meant
'
character instead of `
@@ -186,15 +186,16 @@ func (v *value) define() error { | |||
owner = owner.parent | |||
} | |||
value, exists = v.owner.external.Get(values...) | |||
if exists { |
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.
this changes is not related to this commit
env.go
Outdated
if i == -1 { | ||
return ErrInvalidPair | ||
} | ||
key := string(n[:i]) |
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.
- What do you mean? If you try in bash define variable like
FOO=" BAR"
and printFOO
variable, result will beBAR
without spaces. - Sorry, my fault. I meant
'
character instead of `
No description provided.