Skip to content
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

Add missing $maxsize option to Logrotate::Conf #90

Merged
merged 1 commit into from
Dec 7, 2017

Conversation

achermes
Copy link

This PR re-adds the missing $maxsize option to Logrotate::Conf. This was regressed during a refactor - see #49.

I have only added this line and not tested this as I was not able to get set up for testing this (I ran into gem dependency conflicts).

@bastelfreak
Copy link
Member

Hi @achermes, thanks for the PR. We're happy to assist you. I recommend the following to install the needed gems:

bundle install --path .vendor/ --without system_tests --without development --with release; bundle update; bundle clean

If this doesn't work for you or if you have more questions, please ping us in the #voxpupuli IRC channel on freenode,we're happy to help out.

@juniorsysadmin juniorsysadmin added the bug Something isn't working label Nov 27, 2017
@achermes
Copy link
Author

Ok, after much wrangling with ruby I was able to install the dependencies and run rake test, rake validate and rake spec successfully. I'm not sure what else you'd like me to do ...?

@juniorsysadmin
Copy link
Member

@achermes Are you able to add tests in the /spec directory to cover this new parameter?

@achermes
Copy link
Author

achermes commented Dec 6, 2017

It'll be a while before I have time to engage with this so not in the short term.

I'll point out that this parameter is already referenced in the code (in the templates) :)

@juniorsysadmin juniorsysadmin merged commit bb2ab50 into voxpupuli:master Dec 7, 2017
@juniorsysadmin
Copy link
Member

Fixes #49

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants