ghoneycutt / puppet-module-pam

Puppet module to manage PAM
Other
18 stars 80 forks source link

Add Debian 9 support #194

Closed hdep closed 5 years ago

hdep commented 5 years ago

Here is the rework for adding debian 9 support. Test are ok on my side.

ghoneycutt commented 5 years ago

Thanks @hdep !! We really appreciate your contribution. This PR is almost there there though missing a few things. Take a look at PR #193

hdep commented 5 years ago

here is metadata.json but I don't know how to squash...

ghoneycutt commented 5 years ago

Thanks! I can handle the squash. We still need the testing bits though, just like what was done in that PR for Ubuntu 18.

ghoneycutt commented 5 years ago

@hdep How's it going with the tests? Do you need any help?

hdep commented 5 years ago

Hello, I'm using the module with my debian 9 without any issue so far. Can I do something else ?

let me know

ghoneycutt commented 5 years ago

Thanks for letting us know it is working. We just need the other bits related to testing as shown in PR #193 to get this merged.

hdep commented 5 years ago

Sorry, I don't get what I'm missing for this PR ?

ghoneycutt commented 5 years ago

@hdep check out the files that were modified / created in https://github.com/ghoneycutt/puppet-module-pam/pull/193/files

hdep commented 5 years ago

Here is the change. Honestly I just copy and past value from debian 8 because I'm not sure what this does exactly. So let me know how it goes. Thank you

hdep commented 5 years ago

Hello @ghoneycutt can you please give me some guidelines to fix those issues ? I'm not sure to understand why it fails. Thank you !

ghoneycutt commented 5 years ago

Hi @hdep seems like just a few files have weird endings as noted in the comments. It also needs a nodeset file for beaker as the travis config says to use debian-9 though the file does not exist. You can copy this one https://github.com/ghoneycutt/puppet-module-pam/blob/master/spec/acceptance/nodesets/debian-8.yml to debian-9.yml and just change all the references from 8 to 9. That will likely work fine like that.

ghoneycutt commented 5 years ago

Seems the spacing in the spec files does not match what's in the data files. Suggest copying and pasting the data so they match.

hdep commented 5 years ago

Hi @ghoneycutt I fixed most of the errors, this was painfull :) Can you check other errors ? seems docker related don't know what happen. thanks

ghoneycutt commented 5 years ago

Tests pass! Awesome work!

hdep commented 5 years ago

awesome ! Thanks for the help. Do you know when it will get merged ? just asking because I would like to propose another PR but I didn't use a branch for this one, so I'm blocked I guess I can't fork twice

ghoneycutt commented 5 years ago

I'm going to test this in Vagrant then squash/rebase the commits then merge.

ghoneycutt commented 5 years ago

Thank you @hdep! I've squashed and rebased your work in PR #206 and added Vagrant testing.