-
Notifications
You must be signed in to change notification settings - Fork 71.8k
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
only show the alarm threshold values in the alarm setting when simple mode is enabled #360
Conversation
jasoncalabrese
commented
Jan 22, 2015
@jimsiff you can target your heroku deploy button changes here |
@jasoncalabrese @gregwaehner I created two new Heroku deploy branches. The first uses the same changes from yesterday where ALARM_TYPES was hard set to 'predict' because the BG_* values were hard set to their implied default. That deploy behavior works, but it's not the cleanest because it hard sets variables that don't need to be set. The second branch leaves all alarm related variables blank with simple descriptions that highlight the implied default. I think the second approach is cleaner and the site deploys as originally intended with implied defaults. Or, we can leave these variables completely out of the Heroku deployment and have users manually create and define any optional variables. If we follow that approach to it's logical conclusion, should we then also remove the enable and pushover related variables too? It's a balance between simplicity vs flexibility. See the pics below: Explicitly set variables Implicitly set variables |
The 2nd image with the empty values and defaults in the description make sense to me |
Wip/alarm types update
Heroku Button updates for Alarm Types
PR #364 used the empty values and defaults in the description. |
only show the alarm threshold values in the alarm setting when simple mode is enabled