Skip to content
This repository has been archived by the owner on Jul 11, 2023. It is now read-only.

tests/e2e: Generalize GetEnv #1844

Merged
merged 2 commits into from
Oct 16, 2020
Merged

Conversation

eduser25
Copy link
Contributor

@eduser25 eduser25 commented Oct 14, 2020

Addressing comment from previous PR #1828 (comment)

Affected area:

  • Tests [X]

  • CI System [X]

  • Does this change contain code from or inspired by another project? If so, did you notify the maintainers and provide attribution?
    No

@eduser25 eduser25 requested a review from a team as a code owner October 14, 2020 23:18
@eduser25 eduser25 requested a review from draychev October 14, 2020 23:18
@codecov-io
Copy link

Codecov Report

Merging #1844 into main will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1844      +/-   ##
==========================================
- Coverage   59.21%   59.19%   -0.02%     
==========================================
  Files         125      125              
  Lines        5171     5171              
==========================================
- Hits         3062     3061       -1     
- Misses       2106     2107       +1     
  Partials        3        3              
Impacted Files Coverage Δ
pkg/envoy/route/config.go 95.00% <0.00%> (-0.84%) ⬇️

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 814c2c7...189227d. Read the comment docs.

Comment on lines 76 to 83
func GetEnv(envVar string, defaultValue string) string {
val := os.Getenv(envVar)
if val == "" {
return defaultValue
}
return val
}

Copy link
Member

Choose a reason for hiding this comment

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

This is implemented in at lease one other file:

func GetEnv(envVar string, defaultValue string) string {

Might as well move this under pkg/utils and use that.

Copy link
Contributor Author

@eduser25 eduser25 Oct 15, 2020

Choose a reason for hiding this comment

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

let me try. I hope not to run into cyclic imports.

@eduser25 eduser25 merged commit 321febf into openservicemesh:main Oct 16, 2020
@eduser25 eduser25 deleted the fix-env-e2e branch October 16, 2020 07:49
draychev pushed a commit to draychev/osm that referenced this pull request Oct 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants