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

fixes #4918 - add realm commands to hammer #240

Merged
merged 1 commit into from Sep 1, 2016

Conversation

stbenjam
Copy link
Member

No description provided.

@stbenjam
Copy link
Member Author

[test]



class CreateCommand < HammerCLIForeman::CreateCommand
success_message _("Realm [%{name}] created")
Copy link
Contributor

Choose a reason for hiding this comment

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

This produces messages like this: Realm [realm1] created

Do you want the square brackets in the output?

Copy link
Member Author

@stbenjam stbenjam Aug 17, 2016

Choose a reason for hiding this comment

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

It is in brackets for domains, so it's done here to be consistent

@akofink
Copy link
Contributor

akofink commented Jun 6, 2016

The functionality implemented works for me. The tests make sense and pass locally.

Do you think it would be wise to test that the proper API endpoints are being called?

@stbenjam
Copy link
Member Author

stbenjam commented Jun 6, 2016

Do you think it would be wise to test that the proper API endpoints are being called?

Is there an example in here? This PR is literally a cut and paste job from the domain commands

@akofink
Copy link
Contributor

akofink commented Jun 6, 2016

Yes. They're usually in the functional tests, such as hosts.

Here is a full suite I recently wrote for host_collections.


class InfoCommand < HammerCLIForeman::InfoCommand
output ListCommand.output_definition do
field :realm_proxy_id, _("Realm proxy id")
Copy link
Member

Choose a reason for hiding this comment

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

Just one nitpick: I haven't tested the API, but if it returns realm_proxy_name too, it would be good to use

field nil, _("Realm Proxy"), Fields::SingleReference, :key => :realm_proxy

instead, to show name by default and id when user asks for it (--show-ids).

Copy link
Member

Choose a reason for hiding this comment

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

I checked the api and proxy name isn't contained in the show reply at the moment. Disregard my previous post.

@mbacovsky
Copy link
Member

👍 Thanks to all involved!

@mbacovsky mbacovsky merged commit 4911394 into theforeman:master Sep 1, 2016
@stbenjam stbenjam deleted the 4918 branch September 1, 2016 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants