Hi David, finally back to your patches. There are very minor things but I'm not going to modify them without your consent since they're signed-off.
> From eae56a44d3bf51f8e92f66baef25f2a417f837ce Mon Sep 17 00:00:00 2001 > From: David Carlier <[email protected]> > Date: Fri, 21 Jan 2022 20:46:40 +0000 > Subject: [PATCH 1/3] MEDIUM: deviceatlas new optional data file download > scheduler service. ^^^ Please always place a colon after the subsystem. Also, previously it was referred to as "da", it would be better to stay consistent in order to more easily list all such related changes. > New specialized service to daily handle the update of download file without > interruption of service and to be preemptively started before HAProxy. While I do know from your private explanation that it's an external service, it's not immediately obvious to the reader. Here it would be welcome to mention that it's a standalone utility that accesses a shared memory used by the DA module and explain a little bit how it works (the general principles and choices). And please mention it uses libcurl since it's a new dependency, this may save some trial and errors to those in the process of a bisect for example. Also, I noticed that it doesn't build with the dummy lib. Since it's a standalone tool I don't think it's a problem, but this really needs to be mentioned in the commit message, ideally with a reminder about what's missing (either a link to the download site or an invitation to read build instructions from doc/DeviceAtlas-device-detection.txt). > Signed-off-by: David Carlier <[email protected]> > --- > addons/deviceatlas/Makefile | 49 +++++++++ > addons/deviceatlas/dadwsch.c | 195 +++++++++++++++++++++++++++++++++++ > 2 files changed, 244 insertions(+) > create mode 100644 addons/deviceatlas/Makefile > create mode 100644 addons/deviceatlas/dadwsch.c (...) When I applied it I got these warnings: $ git am 0001-MEDIUM-deviceatlas-new-optional-data-file-download-s.patch Applying: MEDIUM: deviceatlas new optional data file download scheduler service. .git/rebase-apply/patch:14: trailing whitespace. # DEVICEATLAS_SRC : DeviceAtlas API source root path .git/rebase-apply/patch:62: new blank line at EOF. + warning: 2 lines add whitespace errors. One of them is at the end of the line below: > diff --git a/addons/deviceatlas/Makefile b/addons/deviceatlas/Makefile > new file mode 100644 > index 000000000..33dd9434e > --- /dev/null > +++ b/addons/deviceatlas/Makefile > @@ -0,0 +1,49 @@ > +# DEVICEATLAS_SRC : DeviceAtlas API source root path ^ here An easy and convenient way to catch them before committing (or see them after to ease their fixing) is to add this into your $HOME/.gitconfig: [color] ui = true A "git diff" or "git show" will show them on a pretty visible red background, that's how most of us avoid and fix them.. The second one is the empty line at the end of the Makefile. Aside these details it's OK for me. Willy
