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

Allow cron to always mail its output. #124

Merged
merged 5 commits into from
Aug 7, 2018
Merged

Allow cron to always mail its output. #124

merged 5 commits into from
Aug 7, 2018

Conversation

Heidistein
Copy link

I was used to the fact that redhat always mails its output. I enabled
the managed_cron bool, and added the --verbose startup argument (for
debugging) and all mail went away...

Pull Request (PR) description

This Pull Request (PR) fixes the following issues

I was used to the fact that redhat always mails its output. I enabled
the managed_cron bool, and added the --verbose startup argument (for
debugging) and all mail went away...
@Heidistein
Copy link
Author

I'd like some help on how to interpret the test-errors... I don't understand.

@LongLiveCHIEF
Copy link

The current errors you're seeing are a result of a bug with our travis config. It will be fixed as soon as we update modulesync. Once that is merged, you can rebase to re-run CI.

I was used to the fact that redhat always mails its output. I enabled
the managed_cron bool, and added the --verbose startup argument (for
debugging) and all mail went away...
@@ -22,7 +22,8 @@
Stdlib::Filemode $rules_configdir_mode = $logrotate::params::rules_configdir_mode,
String $root_user = $logrotate::params::root_user,
String $root_group = $logrotate::params::root_group,
Array[String] $logrotate_args = []
Array[String] $logrotate_args = [],
Copy link
Member

Choose a reason for hiding this comment

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

Can you please enforce a minimal string length like this:

Array[String[1]]

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I agree that is better. However, it belongs to pull request 80. I will fix it there.
https://github.com/Heidistein/puppet-logrotate/tree/pr80fix

Copy link
Author

@Heidistein Heidistein Jul 30, 2018

Choose a reason for hiding this comment

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

Fixed in PR #127
I added only a trailing comma here, this was unrelated to this change.

@bastelfreak bastelfreak added backwards-incompatible needs-work not ready to merge just yet enhancement New feature or request and removed backwards-incompatible labels Jul 29, 2018
@bastelfreak
Copy link
Member

Hi @Heidistein. thanks for the PR. Can you take a look at the inline comment I made?

@bastelfreak bastelfreak added needs-rebase and removed needs-work not ready to merge just yet labels Aug 6, 2018
@bastelfreak
Copy link
Member

Hi, can you please rebase?

@Heidistein
Copy link
Author

Rebase done.

@@ -78,6 +78,18 @@
with_content(%r{(\/usr\/local\/sbin\/logrotate -s \/var\/lib\/logrotate\/logrotate.status -m \/usr\/sbin\/mailer \/usr\/local\/etc\/logrotate.conf 2>&1)})
}
end

context 'With additional arguments' do
let(:pre_condition) { 'class {"::logrotate": cron_always_output => true}' }
Copy link
Member

Choose a reason for hiding this comment

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

FYI the :: aren't needed in front of logrotate

@bastelfreak
Copy link
Member

Thanks!

@bastelfreak bastelfreak merged commit 04f1b28 into voxpupuli:master Aug 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants