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

[V2V] Remove SetDescription state #509

Merged
merged 2 commits into from
Mar 7, 2019
Merged

[V2V] Remove SetDescription state #509

merged 2 commits into from
Mar 7, 2019

Conversation

ghost
Copy link

@ghost ghost commented Feb 27, 2019 •
edited by simaishi

SetDescription state relies on ovirtsdk4 gem to set the VM description. However, the VM identification fails if the VM name contains a space. As this method isn't used for migration to OpenStack, it also creates inconsistency in migration workflow. It was decided to remove it to fix the space character issue.

Associated RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1669240

@ghost
Copy link
Author

ghost commented Feb 27, 2019

@miq-bot add-label transformation,bug,hammer/yes
@miq-bot add-reviewer mkanoor
@miq-bot add-reviewer gmcculloug

@coveralls
Copy link

coveralls commented Feb 27, 2019 •
edited

Pull Request Test Coverage Report for Build 2783

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.906%
Totals Coverage Status
Change from base Build 2781: 0.0%
Covered Lines: 2819
Relevant Lines: 2909
💛 - Coveralls

@miq-bot
Copy link
Member

miq-bot commented Mar 4, 2019

This pull request is not mergeable. Please rebase and repush.

gmcculloug
gmcculloug reviewed Mar 4, 2019
View reviewed changes
Copy link
Member

@gmcculloug gmcculloug left a comment

Choose a reason for hiding this comment

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

@fdupont-redhat You are removing the RedHat Utils vm_set_description method which is still referenced but no longer called here: https://github.com/ManageIQ/manageiq-content/blob/master/content/automate/ManageIQ/Transformation/Infrastructure/VM/Common.class/__methods__/setdescription.rb#L17

Should the setdescription method be removed in this PR as well?

@ghost ghost force-pushed the v2v_remove_setdescription branch from 7d56624 to ade0264 Compare March 5, 2019 07:25
@miq-bot
Copy link
Member

miq-bot commented Mar 5, 2019

Checked commits fabiendupont/manageiq-content@3d77945~...ade0264 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
0 files checked, 0 offenses detected
Everything looks fine. ⭐

@ghost
Copy link
Author

ghost commented Mar 5, 2019

@gmcculloug you're right, there's some leftover here. I removed setdescription.{rb,yaml} files, since the state machine doesn't call them anymore.

@gmcculloug gmcculloug merged commit 3e72681 into ManageIQ:master Mar 7, 2019
@gmcculloug gmcculloug added this to the Sprint 107 Ending Mar 18, 2019 milestone Mar 7, 2019
@ghost ghost deleted the v2v_remove_setdescription branch March 7, 2019 22:54
simaishi pushed a commit that referenced this pull request Apr 4, 2019
@gmcculloug @simaishi
Merge pull request #509 from fdupont-redhat/v2v_remove_setdescription
840b794 
[V2V] Remove SetDescription state

(cherry picked from commit 3e72681)

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

simaishi commented Apr 4, 2019

Hammer backport details:

$ git log -1
commit 840b7949023c33e519e99943d9e67482a3775e48
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Thu Mar 7 17:28:03 2019 -0500

    Merge pull request #509 from fdupont-redhat/v2v_remove_setdescription
    
    [V2V] Remove SetDescription state
    
    (cherry picked from commit 3e726817190816aea92f2357a136d536997ecaeb)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1678385

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkanoor mkanoor Awaiting requested review from mkanoor

Assignees
No one assigned
Labels
bug hammer/backported v2v
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@coveralls @miq-bot @simaishi @gmcculloug @fabiendupont