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 WIDGET_TYPES, SINGULAR_WIDGET_TYPES from UiConstants #2025

Merged
merged 2 commits into from
Aug 29, 2017

Conversation

europ
Copy link
Member

@europ europ commented Aug 28, 2017

Issue: #1661

Constant SINGULAR_WIDGET_TYPES was replaced with same constant WIDGET_TYPES. Definition of constant SINGULAR_WIDGET_TYPES was removed from UiConstants.

Definition of constant WIDGET_TYPES was removed from UiConstants and moved to ReportController. Prefix ReportController:: was added to every occurrence of WIDGET_TYPES.

@martinpovolny
Copy link
Member

The constants are not the same. It's singular vs plural. There's a hint in the name SINGULAR_ ;-)

Copy link
Member

@martinpovolny martinpovolny left a comment

Choose a reason for hiding this comment

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

singular vs plural

@europ europ force-pushed the remove-ui-constants-41 branch 2 times, most recently from 514e7e5 to 64699b1 Compare August 29, 2017 07:53
@martinpovolny
Copy link
Member

Please, fix the rubocop warning.

@europ europ force-pushed the remove-ui-constants-41 branch from 64699b1 to 2b2ee1c Compare August 29, 2017 10:55
@miq-bot
Copy link
Member

miq-bot commented Aug 29, 2017

Checked commits europ/manageiq-ui-classic@7664522~...2b2ee1c with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks fine. 👍

@europ
Copy link
Member Author

europ commented Aug 29, 2017

@martinpovolny fixed

@martinpovolny martinpovolny added this to the Sprint 68 Ending Sep 4, 2017 milestone Aug 29, 2017
@martinpovolny martinpovolny merged commit 2a760e2 into ManageIQ:master Aug 29, 2017
@europ europ deleted the remove-ui-constants-41 branch August 29, 2017 15:20
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.

3 participants