duct-framework / core

The core library of the Duct framework
76 stars 21 forks source link

Fix default value for Boolean env #17

Closed GaspardP closed 6 years ago

GaspardP commented 6 years ago

This changes allows the use of true as default value for Boolean. It only use then default value when the env variable is nil. I extracted the System/getenv to make it possible to test the function without setting a special environment. I don't know if there is a better way to do that.

weavejester commented 6 years ago

Thanks for the patch. Rather than using a private getenv, perhaps instead something like:

(def ^:dynamic *env*
  (into {} (System/getenv)))
GaspardP commented 6 years ago

No problem, thank you for taking the time to review the PR. Should I squash the commits or would you rather squash-merge it directly?

weavejester commented 6 years ago

Thanks for the update. This looks fine to merge. Can you squash the commits, since Github removes the merge information when I do it through that? (I prefer to have a clean commit history and a merge commit that links back to the PR).

GaspardP commented 6 years ago

Thank you for merging my changes and thank you for all the great libraries.