Closed ThinkChaos closed 4 years ago
Thanks for sending these patches!
I'm making my way through them, starting with the log ones; I'll post something more detailed soon (either today or tomorrow).
Just want to give you a heads-up that I will probably make a release without these today, because a few changes have accumulated, and I prefer if this patch series receives a bit more coverage (log changes tend to have surprising implications), I hope you don't mind.
Thanks again!
Thanks for the reply. Sure take your time!
I just merged this into the next
branch.
I did some amends: fixed the build, extended the commit messages, adjusted to the logs option API (that I changed in the other pull request), and added tests. This is noted in the commit messages, for transparency.
I'm going to let these sit in the next
branch for a little while, since I want to make sure they get a bit more exposure. Sometimes changes like this have unexpected side-effects so this is just in case we need to do some further minor amends, although I don't expect to.
Please let me know if you have any comments or suggestions.
Thanks again for the patches!
Awesome! I'll be running the next branch for now and report if I run into anything weird.
Sorry for not taking the time to write the tests I haven't looked into those at all for chasquid to be honest, and yeah my commit messages could be clearer 😅
Thanks, that's much appreciated!
And no worries, I understand not everyone has the time to write tests, or to do a few iterations of code reviews; all contributions are welcome :)
I would like to be able to use logrotate to manage chasquids logs, but there was no way to tell chasquid to flush and reopen its log file. I saw that your log package has a Reopen function but is marked as experimental. I went ahead and used it to implement log reopening here.
I ported maillog to use your log package to be able to reopen it. To do so I had to modify log to add output format configuration, see https://github.com/albertito/log/pull/2.