Skip to content
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

Remove TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS from UiConstants #1821

Merged
merged 2 commits into from
Aug 14, 2017

Conversation

europ
Copy link
Member

@europ europ commented Aug 3, 2017

Issue: #1661

Constants TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS have been removed from UiConstants. Definitions of constants TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS were moved to ReportHelper. Prefix ReportHelper:: was added to every occurrence of these constants.

Cloud Intel -> Reports -> Schedules -> Copy of ChargeBack by Project -> Configuration -> Edit this Schedule

image

@europ
Copy link
Member Author

europ commented Aug 3, 2017

@miq-bot assign @romanblanco

@europ europ force-pushed the remove-ui-constants-21 branch from c6215db to e06db30 Compare August 3, 2017 14:08
Copy link
Member

@romanblanco romanblanco left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@europ isn't manageiq-ui-classic/app/helpers/report_helper/timer.rb better place for the constants?

@europ europ changed the title Remove TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS from UiConstants [WIP] Remove TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS from UiConstants Aug 3, 2017
@miq-bot miq-bot added the wip label Aug 3, 2017
@europ
Copy link
Member Author

europ commented Aug 4, 2017

Is manageiq-ui-classic/app/controllers/mixins/time_helper.rb a better place than manageiq-ui-classic/app/helpers/report_helper/timer.rb for these constants?

/cc @martinpovolny @romanblanco

@martinpovolny
Copy link
Member

view_helper is included everywhere and the constants are only relevant for reports. That means that @romanblanco is right.

@europ isn't manageiq-ui-classic/app/helpers/report_helper/timer.rb better place for the constants?

…MONTHS" have been moved from UiConstants to ViewHelper
@europ europ force-pushed the remove-ui-constants-21 branch from e06db30 to 2f85d49 Compare August 14, 2017 08:01
@europ europ force-pushed the remove-ui-constants-21 branch from 2f85d49 to d0ebb4b Compare August 14, 2017 08:06
@europ europ changed the title [WIP] Remove TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS from UiConstants Remove TIMER_DAYS,TIMER_HOURS,TIMER_WEEKS,TIMER_MONTHS from UiConstants Aug 14, 2017
@miq-bot miq-bot removed the wip label Aug 14, 2017
@miq-bot
Copy link
Member

miq-bot commented Aug 14, 2017

Checked commits europ/manageiq-ui-classic@d577f5d~...d0ebb4b with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 2 offenses detected

app/helpers/report_helper/timer.rb

@martinpovolny martinpovolny added this to the Sprint 67 Ending Aug 21, 2017 milestone Aug 14, 2017
@martinpovolny martinpovolny merged commit e6c3761 into ManageIQ:master Aug 14, 2017
@europ europ deleted the remove-ui-constants-21 branch August 14, 2017 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants