Skip to content
This repository was archived by the owner on Feb 19, 2024. It is now read-only.

Conversation

@petems
Copy link
Contributor

@petems petems commented Feb 26, 2016

Moves gem repo into new defined type

I'm going to refactor the others, but I thought I'd make the changes in manageable chunks

Also added working rspec tests for the new gem_repo defined type

I ran the test-kitchen tests and everything passed, so functionality is the same 👍

@petems
Copy link
Contributor Author

petems commented Mar 29, 2016

@ice799 any chance of a review? 👍

@ice799
Copy link
Contributor

ice799 commented Apr 7, 2016

sorry for the delay! i'm going to take a look now.

@ice799
Copy link
Contributor

ice799 commented Apr 7, 2016

the changes look fine to me, thanks a lot for your work on this and your patience with my slow review. i'm going to re-run the specs one more time right now just to be safe and once they pass I'll merge this change.

@ice799
Copy link
Contributor

ice799 commented Apr 7, 2016

@petems hmmm.... I'm getting a kitchen failure for default-centos-510: https://gist.github.com/ice799/44ece482ad31e6b2fad5f7b7eb4f89bd

i'm investigating now

'RedHat', 'redhat', 'CentOS', 'centos', 'Amazon', 'Fedora', 'Scientific', 'OracleLinux', 'OEL': {

$majrel = $::osreleasemaj
if $::pygpgme_installed == false {
Copy link
Contributor

Choose a reason for hiding this comment

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

it appears that this check is failing, for some reason, on default-centos-510

Copy link
Contributor

Choose a reason for hiding this comment

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

ah, yes, this is because $::pygpgme_installed is a string, not a truthy value. looks like in the refactor it got switched to a truthy. there was a reason why i made $::pygpgme_installed be a string instead of a truthy originally. it was something about facter functions, but i can't remember why.

if you commit a change to change this back to a string comparison, that should fix the spec and i can merge this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 👍

* Also adds coverage class
@petems petems force-pushed the refactor_gem_repo_into_defined_type branch from ef108c6 to 79cc043 Compare April 8, 2016 15:17
@ice799 ice799 merged commit d67cd65 into computology:master Apr 8, 2016
@petems petems deleted the refactor_gem_repo_into_defined_type branch April 8, 2016 20:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants