missing dependency in metdata.json (theforeman-git) about puppet-puppet

[term1n4l]

given time, I may be able to create a PR for this, but putting the issue in here for awareness while I am thinking of it

[ekohl]

It's mentioned in https://github.com/theforeman/puppet-puppet#git-repo-support that it's needed. Soft dependencies in Puppet aren't great to say the least.

Perhaps it would be better to have something like extlib::has_module. So we can change this part here:

puppet-puppet/manifests/server/config.pp

Lines 253 to 254 in 01a716c

	if $puppet::server::git_repo {
	include git

To

  if $puppet::server::git_repo {
    unless extlib::has_module('theforeman/git') {
      fail('theforeman/git module is not present')
    }
    include git

Alternatively you could use load_module_metadata to load the metadata and verify the version is compatible. I can also imagine has_module could be extended with another argument to verify the version.

[term1n4l]

Making it a hard dependency means it should to be visible to the end user also. Without having it visible in the dependencies on the forge it might be missed. By all means make it a hard dependency, but please consider adding it to the module metadata so it's picked up by the forge as well as some people will only do a cursory scan of the README and still miss that it has undefined dependencies.