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

[MODULES-2048] Add a validate_supported_os() function #457

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@raphink
Member

raphink commented May 18, 2015

No description provided.

@nibalizer

This comment has been minimized.

Show comment
Hide comment
@nibalizer

nibalizer May 21, 2015

Contributor

I'm thinking this isn't a good idea. There aren't docs so I'm not entirely sure what the usecase is. However, I don't see any advantage over putting a series of case statements with fail() line in params.pp or something. Determining os suitability is essentially walking a little tree down the paths you need to evaluate. Thus function has no recursive component, and I'm not sure it should. It also has the problem that it requires a module to be listed as supported, that means that nothing can work on new releases of distros until the module author gets around to updating metadata.json

-1

Contributor

nibalizer commented May 21, 2015

I'm thinking this isn't a good idea. There aren't docs so I'm not entirely sure what the usecase is. However, I don't see any advantage over putting a series of case statements with fail() line in params.pp or something. Determining os suitability is essentially walking a little tree down the paths you need to evaluate. Thus function has no recursive component, and I'm not sure it should. It also has the problem that it requires a module to be listed as supported, that means that nothing can work on new releases of distros until the module author gets around to updating metadata.json

-1

@igalic

This comment has been minimized.

Show comment
Hide comment
@igalic

igalic May 26, 2015

Member

the "documentation" (and further discussion) is in the Jira.


okay, so the author has to update the metadata.json, but when the fail() is in params.pp it's okay?

what?

Member

igalic commented May 26, 2015

the "documentation" (and further discussion) is in the Jira.


okay, so the author has to update the metadata.json, but when the fail() is in params.pp it's okay?

what?

@nibalizer

This comment has been minimized.

Show comment
Hide comment
@nibalizer

nibalizer May 26, 2015

Contributor

@igalic if you have in params.pp:

case $::osfamily {
  /Debian/: { stuff }
  /RedHat/: { stuff }
  default: { fail("wont work here")}
}

Then this will work on all fedoras and all centoses. Under this patch, any new centos that comes out will need to be explicitly added to manifest.json. Its a question of implicitly supporting versus implicitly unsupporting, imho

Contributor

nibalizer commented May 26, 2015

@igalic if you have in params.pp:

case $::osfamily {
  /Debian/: { stuff }
  /RedHat/: { stuff }
  default: { fail("wont work here")}
}

Then this will work on all fedoras and all centoses. Under this patch, any new centos that comes out will need to be explicitly added to manifest.json. Its a question of implicitly supporting versus implicitly unsupporting, imho

@igalic

This comment has been minimized.

Show comment
Hide comment
@igalic

igalic May 27, 2015

Member

@nibalizer from what i gather, from @raphink's patch, only if those are actually mentioned in metadata.json, will they be evaluated.

Member

igalic commented May 27, 2015

@nibalizer from what i gather, from @raphink's patch, only if those are actually mentioned in metadata.json, will they be evaluated.

@nibalizer

This comment has been minimized.

Show comment
Hide comment
@nibalizer

nibalizer May 28, 2015

Contributor

How about building a function that loads metadata.json wholesale? Then any user can validate however they need. Also can infer module version and author.

Example

$metadata = load_metadata_json($mod_name)

// do something with $metadata['something']['something']
Contributor

nibalizer commented May 28, 2015

How about building a function that loads metadata.json wholesale? Then any user can validate however they need. Also can infer module version and author.

Example

$metadata = load_metadata_json($mod_name)

// do something with $metadata['something']['something']
@igalic

This comment has been minimized.

Show comment
Hide comment
@igalic

igalic Jun 1, 2015

Member

related to this discussion is the one going on in #442

Member

igalic commented Jun 1, 2015

related to this discussion is the one going on in #442

@puppet-community-ci

This comment has been minimized.

Show comment
Hide comment
@puppet-community-ci

puppet-community-ci Jun 1, 2015

The result of the test was: PASS
Details at http://planck.nibalizer.com/buildlogs/puppetlabs+puppetlabs-stdlib+457+1433158434+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

puppet-community-ci commented Jun 1, 2015

The result of the test was: PASS
Details at http://planck.nibalizer.com/buildlogs/puppetlabs+puppetlabs-stdlib+457+1433158434+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

@puppet-community-ci

This comment has been minimized.

Show comment
Hide comment
@puppet-community-ci

puppet-community-ci Jun 2, 2015

The result of the test was: FAIL
Details at http://ci.puppet.community/buildlogspuppetlabs+puppetlabs-stdlib+457+1433246524+FAIL

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

puppet-community-ci commented Jun 2, 2015

The result of the test was: FAIL
Details at http://ci.puppet.community/buildlogspuppetlabs+puppetlabs-stdlib+457+1433246524+FAIL

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

@puppet-community-ci

This comment has been minimized.

Show comment
Hide comment
@puppet-community-ci

puppet-community-ci Jun 3, 2015

The result of the test was: PASS
Details at http://ci.puppet.community/buildlogs/puppetlabs+puppetlabs-stdlib+457+1433353844+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

puppet-community-ci commented Jun 3, 2015

The result of the test was: PASS
Details at http://ci.puppet.community/buildlogs/puppetlabs+puppetlabs-stdlib+457+1433353844+PASS

I am a beta ci bot. I am probably lying to you.
You can contact nibalizer for more details.

@hunner

This comment has been minimized.

Show comment
Hide comment
@hunner

hunner Jul 9, 2015

Member

Due to inactivity and not coming to a consensus, I'm closing this PR. Feel free to reopen it with further comments.

Member

hunner commented Jul 9, 2015

Due to inactivity and not coming to a consensus, I'm closing this PR. Feel free to reopen it with further comments.

@hunner hunner closed this Jul 9, 2015

@nibalizer

This comment has been minimized.

Show comment
Hide comment
@nibalizer

nibalizer Jul 9, 2015

Contributor
Contributor

nibalizer commented Jul 9, 2015

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