-
Notifications
You must be signed in to change notification settings - Fork 900
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
Retirement warn in days only #18637
Retirement warn in days only #18637
Conversation
@miq-bot assign @tinaafitz |
hey @eclarizio could you please 👀 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change but otherwise I think we're good here 👍
ea7e036
to
6f9461c
Compare
anyone want to merge this? |
6f9461c
to
e330ef6
Compare
@gmcculloug hi, could you please look at this again? |
@@ -51,7 +48,10 @@ def retire_on_date=(value) | |||
|
|||
def retirement_warning | |||
warn_value = parse_retirement_warn | |||
@attributes[:retirement_warn] = warn_value if warn_value | |||
if warn_value | |||
days_between_warn_and_retirement = (@attributes[:retires_on] - warn_value) / 1.day |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@d-m-u Did you have thoughts on adding to_i
to the first part of the equation as described in #18637 (review) ?
e330ef6
to
3320319
Compare
Checked commits d-m-u/manageiq@0b5d323~...3320319 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0 |
Since we only support days for retirement_warn, the PR that added the hour granularity needed to have those fields removed and remaining fields needed to be changed to make sure that we're staying true to storing an int in that field as a count of days like the UI sets.
So all these fields should set an int that reflects days before retirement.
Fix for #18618.
https://bugzilla.redhat.com/show_bug.cgi?id=1695219 is the bz.