Skip to content

Show tag values in list --active #23

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
Merged
merged 2 commits into from
Jun 5, 2016
Merged

Show tag values in list --active #23

merged 2 commits into from
Jun 5, 2016

Conversation

justinstoller
Copy link
Member

@justinstoller justinstoller commented Jun 2, 2016 •
edited
Loading

If tags exist this will add them as comma-space separated list of {key}: {value} pairs after the currently listed metadata. If there aren't tags for the vm there should be no change to the output.

I enable travis on my fork and tested out a couple different combinations. You can see my version of this PR here: justinstoller#1 Hopefully that info is helpful?

justinstoller
justinstoller reviewed Jun 2, 2016
View reviewed changes
spec/vmfloaty/utils_spec.rb
@@ -31,4 +31,61 @@
expect(Utils.generate_os_hash(host_arg)).to be_empty
end
end

describe '#prettyprint_hosts' do
Copy link
Member Author

Choose a reason for hiding this comment

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

There's some benefit here in specifying the desired output format, but honestly this test is going to be super brittle, so I don't know if it - cost vs benefit wise - is going to be very valuable. Happy to remove it, I just didn't feel like I could submit a patch without a test around the change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah....I feel like that's fine for the most part. I'm not too experienced in webmock unit tests checking stdout from puts but I feel like it should be ok? Is it brittle just because the moment someone changes how the prettyprint hosts method works it'll break? or because there's the potential for other things to appear in stdout while running unit tests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Cool. I agree its probably fine, though brittle for those reasons. Thanks!

@briancain
Copy link
Contributor

@justinstoller thanks! I'll check this out tomorrow. Looks good otherwise, also woo tests!

@briancain
Copy link
Contributor

Woo! Looks good to me:

brian@yocalhost:vmfloaty % floaty modify --tags '{"tag":"myvalue"}' qf8dyzx0r1itkl9                                                                                                                                                                                                                              ±[SHOW-TAGS]
Successfully modified vm qf8dyzx0r1itkl9.
brian@yocalhost:vmfloaty % floaty list -a                                                                                                                                                                                                                                                                        ±[SHOW-TAGS]
Running VMs:
- qf8dyzx0r1itkl9.delivery.puppetlabs.net (centos-7-x86_64, 0.01/12 hours, tag: myvalue)

If you don't mind, it would be great if you'd squash the commits down like you mentioned! And then I'd be happy to merge :) 👍

@justinstoller justinstoller changed the title [wip] Show tag values in list --active Show tag values in list --active Jun 3, 2016
@briancain briancain merged commit b9781e5 into puppetlabs:master Jun 5, 2016
@briancain briancain mentioned this pull request Jun 5, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@justinstoller @briancain