anaconda / ae5-tools

A command-line tool for scripting AE5 actions
https://www.anaconda.com/enterprise/
BSD 3-Clause "New" or "Revised" License
9 stars 8 forks source link

Added Functions For Loading AE5 Secrets and Manipulating Environment Variables #162

Closed joshburt closed 1 year ago

joshburt commented 1 year ago

What

Why

How was this tested?

jlstevens commented 1 year ago

Looks good!

The only feedback I have is that I don't feel the demand_env_var_as_int and demand_env_var_as_float functions do enough to be justified (inlining their contents is just as clear). The one function of this type I would keep is demand_env_var_as_bool as it does contain more logic.

It is also possible I don't understand how you are using these functions and that with some documentation I would understand why a named function is easier than using float(demand_env_var(name=name)) (for instance).

joshburt commented 1 year ago

Thats fair enough. :) Less code is generally better as well. I've pulled the float and integer variants out.

Thanks for taking a look!