Code review comment for ~michal-maloszewski99/ubuntu/+source/logwatch:logwatch-lp-1890748-focal

Revision history for this message
Robie Basak (racb) wrote :

This looks good - it looks like it correctly fixes the problem. A few things need fixing though please:

The debian/patches/ directory in this branch contains both focal-logwatch-ifconfig-fix.patch and logwatch-focal-lp1890748-fix.patch. It's not really a problem to rename the patch, but please don't keep the unused patch around. It might not be worth the noise to even rename it even if the new name is an improvement though - keep in mind that the previous failed fix is on the permanent record now.

logwatch 7.5.2-1ubuntu1.4 is now published in focal-proposed so that version number can't be used again. Please add a new changelog entry instead of adjusting the previous one, and explain what you're fixing. It will have to be version 7.5.2-1ubuntu1.5.

In the git commit message, please avoid a title of "Fix the logwatch package". This is implied - of course an SRU to the logwatch package is to fix the logwatch package. So it basically doesn't convey any information. Instead, how about something like "Correctly fall back to ifconfig"?

review: Needs Fixing

« Back to merge proposal