deis / builder

Git server and application builder for Deis Workflow
https://deis.com
MIT License
40 stars 44 forks source link

ref(controller): use the env config instead of directly getting from environment #431

Closed kmala closed 7 years ago

kmala commented 7 years ago

fixes #338

deis-bot commented 7 years ago

@aledbf, @Joshua-Anderson and @bacongobbler are potential reviewers of this pull request based on my analysis of git blame information. Thanks @kmala!

codecov-io commented 7 years ago

Current coverage is 47.53% (diff: 35.71%)

Merging #431 into master will increase coverage by 0.57%

@@             master       #431   diff @@
==========================================
  Files            26         27     +1   
  Lines          1118       1134    +16   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            525        539    +14   
- Misses          562        563     +1   
- Partials         31         32     +1   

Powered by Codecov. Last update 9172e9a...3a6c891

bacongobbler commented 7 years ago

can you try to write unit tests for this if possible? Trying to increase coverage in #437 so it'd be helpful to have this all covered before merging.

kmala commented 7 years ago

@bacongobbler i had added test.Can you review it again.

bacongobbler commented 7 years ago

LGTM even without the change. This is partially blocking #437 as it touches on some of the code I'm directly testing in build.go (in a good way) :+1: