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

Change saver_strategy value to String #235

Merged
merged 1 commit into from
Apr 10, 2018

Conversation

Fryguy
Copy link
Member

@Fryguy Fryguy commented Apr 9, 2018

Symbols in configuration values are problematic because Symbols do not
roundtrip through JSON. Since the API now exposes Settings, it's not
possible to set Symbols as values. The saver_strategy was fixed in
ManageIQ/manageiq#17168 to support String
values, and the next step is to remove all Symbols from the Settings.

ManageIQ/manageiq#17201
https://bugzilla.redhat.com/show_bug.cgi?id=1558031

@Ladas Please review

@Fryguy Fryguy assigned Glutexo and Ladas and unassigned Glutexo Apr 9, 2018
Symbols in configuration values are problematic because Symbols do not
roundtrip through JSON.  Since the API now exposes Settings, it's not
possible to set Symbols as values.  The saver_strategy was fixed in
ManageIQ/manageiq#17168 to support String
values, and the next step is to remove all Symbols from the Settings.

ManageIQ/manageiq#17201
https://bugzilla.redhat.com/show_bug.cgi?id=1558031
@miq-bot
Copy link
Member

miq-bot commented Apr 9, 2018

Checked commit Fryguy@2b2a250 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
4 files checked, 1 offense detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

👍

@Ladas Ladas merged commit 3f32cf7 into ManageIQ:master Apr 10, 2018
@Ladas Ladas added this to the Sprint 83 Ending Apr 9, 2018 milestone Apr 10, 2018
@Fryguy Fryguy deleted the stringify_saver_strategy branch April 10, 2018 14:34
@cben
Copy link

cben commented Jan 21, 2019

Can this be gaprindashvili/yes?
(to be backported after ManageIQ/manageiq#17168)

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.

5 participants