GNU bug report logs - #45889
Nextcloud Client

Previous Next

Full log

Message #100 received at 45889 <at> debbugs.gnu.org (full text, mbox):

From: Leo Prikler <leo.prikler <at> student.tugraz.at>
To: Raghav Gururajan <rg <at> raghavgururajan.name>, 45889 <at> debbugs.gnu.org
Subject: Re: Nextcloud Client (v15)
Date: Tue, 09 Mar 2021 08:03:08 +0100

Hi Raghav,

> Subject: [PATCH 2/3] gnu: Add qtsolutions.
> +      (version
> +       (git-version "0" revision commit))

> +         (file-name
> +          (git-file-name name version))

> +                 (("qt5")
> +                  "qt")

> +                 (("-head")
> +                  "")

> +                 (("SUBDIRS\\+=examples")
> +                  ""))
Try not to arbitrarily use too many new lines.  If you break Scheme
code in such a manner for no good reason, it will look odd.

> +                 (("toAscii")
> +                  "toLatin1"))
Why not to UTF-8?  Is the input already UTF-8 and we want to convert it
to a single-byte character set?

> +                 (("\\$\\$PWD")
> +                  (assoc-ref outputs "out")))
You should install the shared libraries in the install phase.

> +                         (list
> +                          "qtlockedfile"
> +                          "qtpropertybrowser"
> +                          "qtservice"
> +                          "qtsingleapplication"
> +                          "qtsoap")
Use '(...) for fixed input.

> Subject: [PATCH 3/3] gnu: Add nextcloud-client.
> +         (commit
> +          (string-append "v" version)))

> +                 (("    \\.\\./3rdparty/qtlockedfile/?.*")
> +                  "")
> +                 (("    \\.\\./3rdparty/qtsingleapplication/?.*")
> +                  "")
> +                 (("    \\.\\./3rdparty/kmessagewidget/?.*")
> +                  "")
> +                 (("   list\\(APPEND 3rdparty_SRC
> \\.\\./3rdparty/?.*\\)")
> +                  "")
I'm starting to grow a little suspicious about matching the leading
spaces.  You probably want to lead this (and similar stuff in 0001)
with [ \t]*.

> +                 (("\\$\\{CMAKE_SOURCE_DIR\\}/src/3rdparty/qtlockedf
> ile")
> +                  (string-append (assoc-ref inputs "qtsolutions")
> +                                 "/include/qtlockedfile/"))
> +                 (("\\$\\{CMAKE_SOURCE_DIR\\}/src/3rdparty/qtsinglea
> pplication")
> +                  (string-append (assoc-ref inputs "qtsolutions")
> +                                 "/include/qtsingleapplication/"))
> +                 (("\\$\\{CMAKE_SOURCE_DIR\\}/src/3rdparty/kmessagew
> idget")
> +                  (string-append (assoc-ref inputs "kwidgetsaddons")
> +                                 "/include/KF5/KWidgetsAddons/"))
LGTM, but probably deserves a comment.
Also, you might skip the stuff leading up to /3rdparty, i.e. have
".*/3rdparty/qtlockedfile", etc.  Perhaps this gives you enough wiggle
room to do lockedfile and single application on the same line, but it
doesn't hurt if it doesn't.

> +                 (("\\$\\{synclib_NAME\\}")
> +                  (string-append "${synclib_NAME} "
> +                                 "QtSolutions_LockedFile "
> +                                 "QtSolutions_SingleApplication "
> +                                 "KF5WidgetsAddons")))
Definitely deserves a comment and perhaps a less broad match?

> +               (substitute* '("application.h" "application.cpp")
> +                 (("SharedTools::QtSingleApplication")
> +                  "QtSingleApplication")
> +                 (("slotParseMessage\\(const QString &(msg)?.*\\)")
> +                  "slotParseMessage(const QString &msg)")))
Also deserves a comment about QtSingleApplication differences.

> +                (string-append "set(_install_dir
> \"${CMAKE_INSTALL_PREFIX}"
> +                               "/share/dbus-1/services\")")))
Would the raw string here exceed a line?

> +      ;; All ThirdParty (except QtProgressIndicator)
> +      license:lgpl2.1+
This is now just qtokenizer, right?

Regards,
Leo

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.