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 env var LOGWATCH_SENDER #2362

Merged
merged 4 commits into from
Jan 10, 2022

Conversation

craue
Copy link
Contributor

@craue craue commented Jan 6, 2022

Description

Follow-up to #2360, but using /etc/logwatch/conf/logwatch.conf as config file.

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

This might be considered to be a breaking change if someone relied on the fact that REPORT_SENDER wouldn't influence the sender address for logwatch reports. But when I started using this project, I was actually expecting it would.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (README.md or the documentation under docs/)
  • If necessary I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@georglauterbach georglauterbach added area/configuration (file) area/scripts kind/new feature A new feature is requested in this issue or implemeted with this PR priority/low labels Jan 7, 2022
@georglauterbach georglauterbach added this to the v10.5.0 milestone Jan 7, 2022
@craue
Copy link
Contributor Author

craue commented Jan 7, 2022

I've fixed the issue's description because I just noticed that POSTMASTER_ADDRESS wouldn't bubble to LOGWATCH_SENDER or PFLOGSUMM_SENDER. The documenation for PFLOGSUMM_SENDER was already wrong and I just copied it. Shall I fix the comments for both sender vars?

@georglauterbach
Copy link
Member

I've fixed the issue's description because I just noticed that POSTMASTER_ADDRESS wouldn't bubble to LOGWATCH_SENDER or PFLOGSUMM_SENDER. The documenation for PFLOGSUMM_SENDER was already wrong and I just copied it.

What do you mean by "bubble"?

Shall I fix the comments for both sender vars?

Sure :)

@craue
Copy link
Contributor Author

craue commented Jan 7, 2022

I've fixed the issue's description because I just noticed that POSTMASTER_ADDRESS wouldn't bubble to LOGWATCH_SENDER or PFLOGSUMM_SENDER. The documenation for PFLOGSUMM_SENDER was already wrong and I just copied it.

What do you mean by "bubble"?

I mean that the value of a variable will also set another variable if it's not explicitly set otherwise.

Sure :)

Will do.

@casperklein casperklein marked this pull request as draft January 7, 2022 19:31
@casperklein
Copy link
Member

Don't forget to adjust start-mailserver.sh as well. As of now, the new env can't be used.

@georglauterbach
Copy link
Member

Don't forget to adjust start-mailserver.sh as well. As of now, the new env can't be used.

Isn't it enough that they're set up in _setup_default_vars in setup-stack.sh?

@casperklein
Copy link
Member

I think you are right. I find it always confusing, when default vars are defined in two places 😕

@georglauterbach
Copy link
Member

I think you are right. I find it always confusing, when default vars are defined in two places confused

Very true. I guess the variables in _setup_default_vars need some setup beforehand (like hostname and stuff). That#s why they're located in that specific place.

@georglauterbach georglauterbach marked this pull request as ready for review January 9, 2022 12:57
@craue
Copy link
Contributor Author

craue commented Jan 9, 2022

Not sure about the failing test. Is it related to my changes?

Edit: Interesting... now the failure is gone. 😮

@github-actions
Copy link
Contributor

Documentation preview for this PR is ready! 🎉

Built with commit: 267c872

@georglauterbach
Copy link
Member

Not sure about the failing test. Is it related to my changes?

Edit: Interesting... now the failure is gone. 😮

Sometimes, although not too often, a test is flakey and does not behave well. Restarting the jobs solves these problems.

@georglauterbach georglauterbach merged commit da17e8b into docker-mailserver:master Jan 10, 2022
@georglauterbach georglauterbach linked an issue Jan 10, 2022 that may be closed by this pull request
@craue craue deleted the logwatch-sender-env branch January 10, 2022 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration (file) area/scripts kind/new feature A new feature is requested in this issue or implemeted with this PR priority/low
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to set Logwatch sender address?
5 participants