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 #3036, #12587, #3956 - Ping command #394

Merged
merged 1 commit into from Oct 17, 2019

Conversation

@ofedoren
Copy link
Member

ofedoren commented Oct 12, 2018

Requires theforeman/foreman#6794 and theforeman/hammer-cli#291 to be merged first.

Also there will be need to update foreman_api.json after that PR is merged. Until then tests will be red.

@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from 90e648b to 9e33d87 Feb 5, 2019
@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from 63d98d6 to 4e0c6ca Apr 9, 2019
@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from 4e0c6ca to c6f2018 May 24, 2019
@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from c6f2018 to f8d3233 Sep 9, 2019
@evgeni

This comment has been minimized.

Copy link
Member

evgeni commented Sep 13, 2019

[test hammer]

@evgeni

This comment has been minimized.

Copy link
Member

evgeni commented Sep 13, 2019

@ofedoren I think you need to update the tests to have a 1.24/develop apidoc.json for this to pass.

@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from f8d3233 to a3c96a7 Sep 13, 2019
@ofedoren

This comment has been minimized.

Copy link
Member Author

ofedoren commented Sep 13, 2019

@evgeni, done. The test failures are not related to this PR.

@chris1984

This comment has been minimized.

Copy link
Member

chris1984 commented Oct 14, 2019

@mbacovsky can this get merged, I acked it. Once it is merged here I will merge the hammer-katello one.

Copy link
Member

chris1984 left a comment

Looks fine to me, ACK

@ofedoren ofedoren force-pushed the ofedoren:feat-3036-ping branch from 7f5fe01 to 7425ef8 Oct 14, 2019
@ofedoren

This comment has been minimized.

Copy link
Member Author

ofedoren commented Oct 14, 2019

Also, I've updated foreman_api.json, so test should be green now 🎉.

Copy link
Member

mbacovsky left a comment

Thanks @ofedoren! It works flawlessly with/without updated h-c-k and also with the new API patches Katello/katello#8386, theforeman/foreman#7111. We may need APIdoc updated in tests for 1.24 once they are merged but there is no reason to block this PR anymore. My concerns with API descriptions were fixed. Good to merge. 💯 👍

@mbacovsky mbacovsky merged commit 62ee512 into theforeman:master Oct 17, 2019
2 checks passed
2 checks passed
Redmine issues Valid issues
Details
hammer Build finished. 5355 tests run, 0 skipped, 0 failed.
Details
@mbacovsky mbacovsky added Demo worthy and removed Needs testing labels Oct 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.