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 #24735 - parameters missing from GET hostgroup api #6003

Conversation

Projects
None yet
4 participants
@kgaikwad
Copy link
Member

commented Aug 28, 2018

Reference PR-4718 which added include parameters change.

@theforeman-bot

This comment has been minimized.

Copy link
Member

commented Aug 28, 2018

Issues: #24735

@kgaikwad kgaikwad force-pushed the kgaikwad:24735_missing_parameters_from_hostgroup_api branch from a3f2ec2 to f442ecd Aug 29, 2018

@kgaikwad kgaikwad changed the title Fixes #24735 - parameters missing from GET hostgroup api Refs #20500, #24735 - parameters missing from GET hostgroup api Aug 29, 2018

@xprazak2

This comment has been minimized.

Copy link
Contributor

commented Aug 29, 2018

Test failures unrelated, the parameters show for me now in the response:

@kgaikwad, could you change the commit message to 'Fixes #24735 ...' please? We use 'Refs' when we do not have a separate ticket for the issue.

@kgaikwad kgaikwad force-pushed the kgaikwad:24735_missing_parameters_from_hostgroup_api branch from f442ecd to 0105f07 Aug 29, 2018

@kgaikwad kgaikwad changed the title Refs #20500, #24735 - parameters missing from GET hostgroup api Fixes #24735 - parameters missing from GET hostgroup api Aug 29, 2018

@kgaikwad

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2018

@xprazak2,

Thank you. Updated the commit message to 'Fixes #24735 ...'.

@xprazak2

This comment has been minimized.

Copy link
Contributor

commented Aug 30, 2018

Nice, could you add a simple test as well so that we can catch possible regressions?

@kgaikwad

This comment has been minimized.

Copy link
Member Author

commented Aug 31, 2018

@xprazak2,

We already have one test case in file test/controllers/api/v2/hostgroups_controller_test.rb.
When I try to run it against develop branch, it is failing on my development setup and same test with this branch, it passes successfully.
I guess, this is what you would like to have in a test similar to above. Please correct me if any other scenario that needs to be handle in a different test case.
or I think I will need to add test case in Katello?

@xprazak2
Copy link
Contributor

left a comment

Ah, so we have tests already, I did not notice. I think this is good to go now.

@xprazak2 xprazak2 removed the Needs testing label Aug 31, 2018

@mmoll mmoll merged commit 79b5e84 into theforeman:develop Sep 1, 2018

6 of 7 checks passed

foreman Build finished. 35404 tests run, 5 skipped, 2 failed.
Details
Hound No violations found. Woof!
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
katello Build finished. 3951 tests run, 9 skipped, 0 failed.
Details
prprocessor Commit message style is correct
Details
upgrade Build finished. No test results found.
Details
@mmoll

This comment has been minimized.

Copy link
Member

commented Sep 1, 2018

merged, thanks @kgaikwad!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.