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

Tag migrated VM. #324

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Tag migrated VM. #324

merged 1 commit into from
Jun 12, 2018

Conversation

lfu
Copy link
Member

@lfu lfu commented Jun 6, 2018

@lfu
Copy link
Member Author

lfu commented Jun 6, 2018

@miq-bot add_label enhancement, transformation

@gmcculloug
Copy link
Member

@fdupont-redhat Please review/test

@@ -28,8 +28,9 @@ def main
exit MIQ_OK
Copy link

Choose a reason for hiding this comment

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

Currently, the code exits right at the beginning of main. If we want the tag assignment to be active, the exit MIQ_OK statement should be placed after source_vm.tag_with(...).

Copy link
Member

Choose a reason for hiding this comment

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

To take this a step further it would be better to remove the exit MIQ_OK line completely and change this method to only do the tagging.

The other logic should either be removed or at the least moved into a new method in this script.

@fdupont-redhat Are there plans to use this logic soon? If not let's remove the unused code.

cc @mkanoor

Copy link

Choose a reason for hiding this comment

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

@gmcculloug, no plan to change it soon, so yes it can be remove. It will be cleaner.

@lfu lfu force-pushed the migrated_policy_1565199 branch from fde0733 to 79dfbbe Compare June 6, 2018 16:40
@miq-bot
Copy link
Member

miq-bot commented Jun 6, 2018

Checked commit lfu@79dfbbe with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍰

@gmcculloug
Copy link
Member

@fdupont-redhat Don't want to lose the question from the hidden comment #324 (comment)

@fdupont-redhat Are there plans to use this logic soon? If not let's remove the unused code.

@ghost
Copy link

ghost commented Jun 6, 2018 via email

@@ -28,8 +28,9 @@ def main
exit MIQ_OK
Copy link

Choose a reason for hiding this comment

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

@gmcculloug, no plan to change it soon, so yes it can be remove. It will be cleaner.

@gmcculloug gmcculloug merged commit d7d3f5d into ManageIQ:master Jun 12, 2018
@gmcculloug gmcculloug added this to the Sprint 88 Ending Jun 18, 2018 milestone Jun 12, 2018
simaishi pushed a commit that referenced this pull request Jun 12, 2018
@simaishi
Copy link
Contributor

Gaprindashvili backport details:

$ git log -1
commit 9065f2754cb1b97a12709ce2624b0bec5dd5d29a
Author: Greg McCullough <[email protected]>
Date:   Tue Jun 12 08:59:12 2018 -0400

    Merge pull request #324 from lfu/migrated_policy_1565199
    
    Tag migrated VM.
    (cherry picked from commit d7d3f5d13e8326c9b2ceab1f59df5de58130d903)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1590430

@lfu lfu deleted the migrated_policy_1565199 branch September 29, 2018 14:33
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.

4 participants