closes #46

[majormoses]

Fixes

#46

General

• Update Changelog following the conventions laid out on Keep A Changelog
• Update README with any necessary configuration snippets
• Binstubs are created if needed
• RuboCop passes
• Existing tests pass

Purpose

Known Compatablity Issues

remove hardcoded values when using --used

[sstarcher]

To keep with the original functionality would it make sense for when --used is supplied to do

config[:warn] = 100 - config[:warn]

[majormoses]

@sstarcher the problem is that if --used is specified we are now overwriting what they may have specified...I think its just better to say if you are using --used which is not default you should specify your own values...

[sstarcher]

I think the best design would to remove the --used option entirely.

[majormoses]

@sstarcher not sure I agree, what is your rationale? If we were to go with best case design I would argue to remove behavior of --free rather than --used; most checks I have seen are more geared to crossing a threshold as opposed to under a threshold (not that they don't exist). Maybe its just me but when thinking about some number of a particular resource I tend to think in terms of in use rather than how much is free (yes its a subtle difference).

If we are to fix the reported bug without removing functionality I think the proposed solution makes sense. If we are ok with removing functionality I think it makes more sense to to the reverse of what you proposed and do a major rev to avoid it breaking people using existing functionality.

[sstarcher]

I don't agree that your changes does not break existing users. If anyone is currently using the plugin and is using --used after your change they will always see alerts as they will have thresholds of 5 and 10. My recommendation above for

config[:warn] = 100 - config[:warn]

[majormoses]

@sstarcher sorry I have been MIA (traveling for work), after reading your thoughts I agree that pretty much any good change is a breaking change at this point because the original feature was flawed. the reason I dont like config[:warn] = 100 - config[:warn] is because it forces you to think of things in terms of free even when using --used which is I think more confusing. I think the best case scenario at this point is to accept this removal of overwriting of cli options and make this a major rev change. While this has the potential of setting off a few alerts for people it would be a major version bump so it should not happen to anyone locking the version and I would rather if we are to have a breaking change I would rather it alert falsely then pass silently. This restores the plugin to sane defaults and if you want to do something other than defaults then everything works as intended. if there was a way to check if the value is default or not then we could modify this to be less breaking but I am unfamiliar with a way of doing something like assuming we know the value of the default and hope that if someone changes the default at some point they update both places.

[sstarcher]

@majormoses That sounds reasonable. Update the revision to a major update and I'll merge.

[majormoses]

@sstarcher ready for a review and release, thanks

[majormoses]

@sstarcher can we please get a release as this is a major change?