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.

Sign up for GitHub

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

Jump to bottom

Changes sysprep field to be a hash #35

Merged
merged 1 commit into from
Apr 18, 2017
Merged

Changes sysprep field to be a hash #35

merged 1 commit into from
Apr 18, 2017

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Apr 5, 2017

Data for the sysprep_timezone dialog field is stored differently than other fields. Instead of hashes the data is stored in an array which is causing https://bugzilla.redhat.com/show_bug.cgi?id=1406084.

This changes the field to a hash like the others.

@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 5, 2017

@miq-bot add_label bug
@miq-bot assign @gmcculloug

@d-m-u d-m-u force-pushed the sysprepBug branch 2 times, most recently from 5b87243 to 8d623ce Compare April 5, 2017 04:34
gmcculloug
gmcculloug reviewed Apr 5, 2017
View reviewed changes
app/models/manageiq/providers/vmware/infra_manager/provision_workflow.rb Outdated
"285" => "(GMT+12:00) Fiji Islands, Kamchatka, Marshall Islands",
"290" => "(GMT+12:00) Auckland, Wellington",
"300" => "(GMT+13:00) Nuku'alofa"
}.freeze
Copy link
Member

Choose a reason for hiding this comment

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

@d-m-u This is a good start, but we should keep the single quotes since there is no string interpolation happening here.

The goal is to keep the data as a hash throughout the provisioning workflow (including interactions with the UI) so that the data can be processed from an API call similar to other dialog fields.

gmcculloug
gmcculloug reviewed Apr 5, 2017
View reviewed changes
app/models/manageiq/providers/vmware/infra_manager/provision_workflow.rb Outdated
'285' => "(GMT+12:00) Fiji Islands, Kamchatka, Marshall Islands",
'290' => "(GMT+12:00) Auckland, Wellington",
'300' => "(GMT+13:00) Nuku'alofa"
}.freeze
Copy link
Member

Choose a reason for hiding this comment

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

And while you are at it single quotes would work on the value side of the hash. Note "(GMT+13:00) Nuku'alofa" requires escaping: '(GMT+13:00) Nuku\'alofa'

@miq-bot
Copy link
Member

miq-bot commented Apr 7, 2017

@d-m-u 'gmcculloug' is an invalid assignee, ignoring...

@miq-bot miq-bot added the bug label Apr 7, 2017
@gmcculloug
Copy link
Member

@d-m-u Please squash these commits

@gmcculloug
Copy link
Member

@d-m-u Something went wrong with your rebase. Pulled in some other commits.

@miq-bot
Copy link
Member

miq-bot commented Apr 8, 2017

Checked commit d-m-u@8719627 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

@agrare
Copy link
Member

agrare commented Apr 10, 2017

@gmcculloug will merge on your 👍

@d-m-u
Copy link
Contributor Author

d-m-u commented Apr 10, 2017 •
edited
Loading

I can't test this, I don't have data for the provider.

This was referenced Apr 10, 2017
Closed
Merged
@d-m-u d-m-u changed the title [WIP] Changes sysprep field to be a hash Changes sysprep field to be a hash Apr 10, 2017
@d-m-u d-m-u mentioned this pull request Apr 12, 2017
Closed
@gmcculloug gmcculloug mentioned this pull request Apr 14, 2017
Merged
@gmcculloug
Copy link
Member

@agrare This is good to merge now.

@agrare agrare merged commit 73ef1aa into ManageIQ:master Apr 18, 2017
@agrare agrare added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 18, 2017
simaishi pushed a commit that referenced this pull request Apr 19, 2017
@agrare @simaishi
Merge pull request #35 from d-m-u/sysprepBug
528f17b 
Changes sysprep field to be a hash
(cherry picked from commit 73ef1aa)

https://bugzilla.redhat.com/show_bug.cgi?id=1443247
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 528f17bce76c093d2a35f8d6662532792935f2d1
Author: Adam Grare <[email protected]>
Date:   Tue Apr 18 11:36:08 2017 -0400

    Merge pull request #35 from d-m-u/sysprepBug
    
    Changes sysprep field to be a hash
    (cherry picked from commit 73ef1aac2ea57412012b99cc6a31a81567199e5c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1443247

@simaishi
Copy link
Contributor

Euwe backport (to manageiq repo) details:

 $ git log -1
commit 34645a2b08383b9fe21f5f7325315c3f6aa7d08b
Author: Adam Grare <[email protected]>
Date:   Tue Apr 18 11:36:08 2017 -0400

    Merge pull request #35 from d-m-u/sysprepBug
    
    Changes sysprep field to be a hash
    (cherry picked from commit 73ef1aac2ea57412012b99cc6a31a81567199e5c)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1443248

agrare pushed a commit to agrare/manageiq-providers-vmware that referenced this pull request Apr 15, 2019
@mkanoor
Merge pull request ManageIQ#35 from tinaafitz/generic_statemachine_re…
ca4d43c 
…tire_change

Ansible Playbook Generic State machine retirement change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmcculloug gmcculloug gmcculloug left review comments

Assignees

@gmcculloug gmcculloug

Labels
bug euwe/backported fine/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@d-m-u @miq-bot @gmcculloug @agrare @simaishi