search

edestecd / puppet-clamav

Puppet Module for managing ClamAV
GNU General Public License v3.0
6 stars 59 forks source link

Completed OS support for clamav-milter #41

Closed ubellavance closed 7 years ago

ubellavance commented 7 years ago

Added Ubuntu 12.04, 14.04, 16.04; RHEL6 derivatives.

Fixes #32.

ubellavance commented 7 years ago

Is it possible that the tests fails because Travis-CI are migrating to Trusty? I see that the last tests that passed has run on Precise.

FYI, here's what we can see on a job that ran on Trusty:

This job ran on our Trusty environment, which is gradually becoming our default Linux environment. Read all about this in our blog: Trusty as a default Linux is coming and take note that you can add dist: precise in your .travis.yml file to continue using Precise.

Example: https://travis-ci.org/ubellavance/puppet-clamav/jobs/258788836

edestecd commented 7 years ago

I don't think it's trusty. I had to update some Gemfile and Rakefile settings in other modules to get the tests working again. I'll see about getting that fixed this evening.

ubellavance commented 7 years ago

I've created tests for clamav-milter in a separate branch. Should I create a separate PR or integrate it in this one? https://github.com/ubellavance/puppet-clamav/blob/spec_helper/spec/classes/clamav_milter_spec.rb

edestecd commented 7 years ago

GO ahead and make a separate branch and MR

ubellavance commented 7 years ago

I managed to make the tests work for ruby 1.9.3 by pinning versions. metadata-json-lint's latest version doesn't support 1.9.3.

https://github.com/ubellavance/puppet-clamav/blob/fix_tests_2/Gemfile

The tests are run, but we have puppet-lint errors: https://travis-ci.org/ubellavance/puppet-clamav/jobs/258918796.

I haven't looked at the ruby 2.x test failures yet.

edestecd commented 7 years ago

I'm removing 1.9.3 from the tests as the next major version will drop puppet 3 support. I wouldn't worry about fixing the tests for now. I'll get all that updated this evening then you can rebase.

ubellavance commented 7 years ago

Ok, I deleted my branches and submitted the PR for tes tests.

edestecd commented 7 years ago

I have fixed the tests. Please rebase from master

ubellavance commented 7 years ago

I don't know how to rebase so I deleted my repo, and lost all the changes.

edestecd commented 7 years ago

Yea github is all upset about your missing repo now. Can you make another fork and try adding the changes again? You can look at these PRs and just copy and paste...

You could just do one PR for both the changes and spec tests...

edestecd commented 7 years ago

@ubellavance I'm going to be doing some work here soon and updating for puppet 4 to cut a new version. It would be nice to have your changes for the next version.

ubellavance commented 7 years ago

Sounds good. Do you have a date in mind? I think it's not a good idea to do a single PR for both the code and the tests, because I don't think my tests specs are ok. Unless you say otherwise...

edestecd commented 7 years ago

There is no big rush. I'm going to try to do some work this week or next at the latest.

It is common to add related spec tests with the features they are testing in the same PR. In fact, most supported/approved modules require this now.

I'm not sure if these tests are specifically related to the OS support or just adding Milter. I'm not too picky, so whatever you think. I wouldn't be opposed to separating them into two PRs, if that works better for you.

ubellavance commented 7 years ago

Ok, I think I should be able to look a these next week at the latest as well. The tests are about adding clamav-milter.

ubellavance commented 7 years ago

The PR is done for the clamav-milter full OS support. The tests are already in another PR, using a separate branch from my repo so you should be good merging them. Closing this PR.