-
Notifications
You must be signed in to change notification settings - Fork 897
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
Rename events "ExtManagementSystem Compliance *" -> "Provider Compliance *" #13388
Rename events "ExtManagementSystem Compliance *" -> "Provider Compliance *" #13388
Conversation
…nce *" Low-level names remain extmanagementsystem_compliance_*, should be backward compatible given existing policies with these events.
Checked commit cben@36ca6ea with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1 |
@cben do you think this can be done? These are fixtures, what about existing installations out there? |
AFAIK everything else in DB references the internal_event_names (which I
didn't change),
and the Human Description is only used in UI.
I've confirmed that given an existing policy, after this PR (and possibly
db:seed?) makes the policy display the new event name in UI.
So I *believe* it's safe and backward compatible. But I might be missing
something.
2017-02-01 0:28 GMT+02:00 Federico Simoncelli <[email protected]>:
… @cben <https://github.com/cben> do you think this can be done? These are
fixtures, what about existing installations out there?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#13388 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAQtGFg0Pid4HV75eNYnmFb3PMzPLzh0ks5rX7WagaJpZM4LdrMd>
.
|
LGTM 👍 @gtanzillo do you have any insight on the backward compatibility? (It seems OK but you know more for sure) Since this is impacting UI @h-kataria @dclarizio do you want to review/ack/nack? Assigning to @gtanzillo for backward compatibility comments and review/merge. @miq-bot assign gtanzillo |
Makes sense to me! |
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.
Looks good. Might want to see if there is documentation that need to be updated with this change.
Ping @gmcculloug so can this be merged? This renaming itself doesn't require doc changes, but these events are missing from the list |
These events appeared in euwe, would be nice to call the consitently there too, if there are not concerns. @miq-bot add-label euwe/yes |
Yes, I am good with merging. @gtanzillo Any concerns, if not please merge. |
👍 LGTM |
…ption Rename events "ExtManagementSystem Compliance *" -> "Provider Compliance *" (cherry picked from commit af4f115) https://bugzilla.redhat.com/show_bug.cgi?id=1411364
Euwe backport details:
|
Followup to #11002,
https://bugzilla.redhat.com/show_bug.cgi?id=1411337 (master)
https://bugzilla.redhat.com/show_bug.cgi?id=1411364 (euwe)
Before:
After (this PRs only fixes the event name, icons fixed in UI PR ManageIQ/manageiq-ui-classic/pull/94):
Low-level names remain
extmanagementsystem_compliance_*
, should be backward compatible given existing policies with these events.I thought this csv is loaded on every run, but it seems I needed
rake db:seed
to take effect.