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

Refs #22253 - Allow mixing option sources and validations #402

Merged
merged 2 commits into from
Dec 17, 2018
Merged

Refs #22253 - Allow mixing option sources and validations #402

merged 2 commits into from
Dec 17, 2018

Conversation

tstrachota
Copy link
Member

This patch makes hammer-cli-katello compatible with changes in:
theforeman/hammer-cli#242

Please see the discussion in the PR into hammer core for more details about the functionality that is going to be merged there.

This was referenced Dec 14, 2018
Merged
Merged
@mbacovsky
Copy link
Member

@tstrachota looks good, tests are green locally and runs flawlessly.
It could be useful to add at least a link to option processors docs in core and full processors stack structure to https://github.com/theforeman/hammer-cli-foreman/blob/master/doc/name_id_resolution.md. What do you think?

@tstrachota
Copy link
Member Author

@mbacovsky updated

mbacovsky
mbacovsky approved these changes Dec 17, 2018
View reviewed changes
Copy link
Member

@mbacovsky mbacovsky left a comment

Choose a reason for hiding this comment

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

Thank you @tstrachota for updating the docs!

@mbacovsky
Copy link
Member

👍 I'll merge once @akofink confirms hammer-cli-katello is in a good shape with this change and once the tests are green with core part merged

@tstrachota
Copy link
Member Author

@mbacovsky would you mind pinging him? Since his last comment was "I want to re-review after the dependent changes are finalized." I'm afraid we could end up in a deadlock :)

@mbacovsky
Copy link
Member

[test hammer]

@mbacovsky
Copy link
Member

Tests are green, merging. Thanks @tstrachota, @akofink! 🍻

@mbacovsky mbacovsky merged commit 12fc9d6 into theforeman:master Dec 17, 2018
jturel pushed a commit to jturel/hammer-cli-foreman that referenced this pull request Apr 10, 2019
@jturel
Refs #22253 - Allow mixing option sources and validations (theforeman…
5307525 
…#402)

* Refs #22253 - Allow mixing option sources and validations

(cherry picked from commit 12fc9d6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mbacovsky mbacovsky mbacovsky approved these changes

Assignees
No one assigned
Labels
Needs testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tstrachota @mbacovsky @theforeman-bot