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 #18532 - Better help for unsupported hammer commands #257

Merged
merged 1 commit into from Feb 23, 2018

Conversation

Projects
None yet
4 participants
@ofedoren
Member

ofedoren commented Nov 7, 2017

No description provided.

@tstrachota

This comment has been minimized.

Show comment
Hide comment
@tstrachota

tstrachota Dec 7, 2017

Member

[test]

Member

tstrachota commented Dec 7, 2017

[test]

@mbacovsky

@ofedoren thanks for the patch. I'd prefer to have incompatible resources and actions handled consistently as I suggested in the other comments. As it does not match the requested output, let's see what the reporter (@tstrachota ) thinks.

Show outdated Hide outdated lib/hammer_cli/apipie/command.rb Outdated
@ofedoren

This comment has been minimized.

Show comment
Hide comment
@ofedoren

ofedoren Jan 15, 2018

Member

@mbacovsky I've rephrased as you suggested. Also I think that your rephrased condition checks for a little bit different thing. I was patterned on https://github.com/ofedoren/hammer-cli/blob/29a4335fc88ac77a2aae13adf12ff9d8c67ad8ce/lib/hammer_cli/apipie/command.rb#L52-L54

Member

ofedoren commented Jan 15, 2018

@mbacovsky I've rephrased as you suggested. Also I think that your rephrased condition checks for a little bit different thing. I was patterned on https://github.com/ofedoren/hammer-cli/blob/29a4335fc88ac77a2aae13adf12ff9d8c67ad8ce/lib/hammer_cli/apipie/command.rb#L52-L54

@tstrachota

This comment has been minimized.

Show comment
Hide comment
@tstrachota

tstrachota Jan 26, 2018

Member

@mbacovsky ping. I guess that this is waiting for your response.

Member

tstrachota commented Jan 26, 2018

@mbacovsky ping. I guess that this is waiting for your response.

@mbacovsky

This comment has been minimized.

Show comment
Hide comment
@mbacovsky

mbacovsky Feb 23, 2018

Member

[test]

Member

mbacovsky commented Feb 23, 2018

[test]

@mbacovsky

This comment has been minimized.

Show comment
Hide comment
@mbacovsky

mbacovsky Feb 23, 2018

Member

👍 @ofedoren, thanks for the update. Looks good now.

Member

mbacovsky commented Feb 23, 2018

👍 @ofedoren, thanks for the update. Looks good now.

@mbacovsky mbacovsky merged commit 768325d into theforeman:master Feb 23, 2018

1 check passed

default Job result: SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment