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

Add hammer plugin for foreman_webhooks #977

Merged
merged 1 commit into from
Aug 2, 2021

Conversation

ofedoren
Copy link
Member

No description provided.

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Could you also add an acceptance test for it? https://github.com/theforeman/puppet-foreman/blob/master/spec/acceptance/foreman_cli_plugins_spec.rb should be the place for that.

@ofedoren
Copy link
Member Author

@ekohl, I guess done...

@ekohl
Copy link
Member

ekohl commented Jul 23, 2021

Looks like there is no package on EL8. theforeman/foreman-packaging@b768dc0 probably hasn't triggered a rebuild in koji. This is something the packaging team needs to fix.

And I guess it's not packaged at all on Debian-based distros. Do you want to address that or change the tests to only test on RPMs?

@ekohl
Copy link
Member

ekohl commented Jul 23, 2021

Looks like there is no package on EL8. theforeman/foreman-packaging@b768dc0 probably hasn't triggered a rebuild in koji. This is something the packaging team needs to fix.

http://koji.katello.org/koji/buildinfo?buildID=66308 exists, https://ci.theforeman.org/job/foreman-plugins-nightly-rpm-pipeline/681/ will publish it.

@ofedoren
Copy link
Member Author

And I guess it's not packaged at all on Debian-based distros. Do you want to address that or change the tests to only test on RPMs?

I'll try to setup a Debian VM in the next few days to create a PR for packaging.

@ofedoren
Copy link
Member Author

@ekohl here is packaging PR for Debian: theforeman/foreman-packaging#6952

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I restarted the tests since the packages should be available now.

@ekohl
Copy link
Member

ekohl commented Aug 2, 2021

That passed, thanks!

@ekohl ekohl merged commit ae455be into theforeman:master Aug 2, 2021
@ofedoren
Copy link
Member Author

ofedoren commented Aug 5, 2021

@ekohl, as you mentioned in theforeman/foreman-installer#702 (comment), what branch should I use to open a CP PR against? I need this plugin to be available for installation through installer for 2.5/6.10.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants