From: Thovthe via Guix-patches via <guix-patches@gnu.org>
To: "41040@debbugs.gnu.org" <41040@debbugs.gnu.org>
Subject: [bug#41040] [PATCH] Package Definition for QDirStat
Date: Thu, 04 Jun 2020 07:40:35 +0000 [thread overview]
Message-ID: <Nc6dd4WaVM3RhEkgX7S588RvUDKStBk52aTYK0lPdSpgZDRY7FVK-PxNfNVy8OUaKJcYe9eYszXFCIOS7ppMQvaVdyVxUy-ByiHoCjSTsf0=@protonmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.2005042239310.5735@marsh.hcoop.net>
> Thanks for contributing this patch to Guix! I have some suggestions for
> how it can be cleaned up so that it can be included in the distribution.
Sounds good, I'll try to address those suggestions.
> Guix uses a very structured changelog format for commit messages. I find
> that the best way to figure it out is to look at the existing git log. In
> this case the commit message should be something like:
>
> gnu: Add qdirstat.
>
> * gnu/packages/qdirstat.scm: New file.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
I hope the new one works fine. Do you want me to amend or smudge?
> As you may have guessed from my proposed commit message, when adding a new
> file, you should also add it to the appropriate section in gnu/local.mk
I have done, but I must admit it is somewhat confusing to me that it goes
under GNU_SYSTEM_MODULES.
> > @@ -0,0 +1,46 @@
> > +(define-module (qdirstat)
>
> The module name should be (gnu packages qdirstat) to match the file path
> of the module.
>
> If you'll permit me to speculate a little bit: I suspect that you
> developed this package outside of the main Guix repository.
Done.
It was originally called that but the linter told me I ought to change
it. Did it do that because I worked on this outside of git?
> > - #:use-module (gnu packages qt)
> > - #:use-module (gnu packages compression)
> > - #:use-module (guix build-system gnu)
> > - #:use-module (guix git-download)
> > - #:use-module ((guix licenses) #:prefix license:)
> > - #:use-module (guix packages))
>
> Since this is a new file, let's go ahead and sort these alphabetically.
I have now but I'm not sure how to treat parens so the `licenses` line is at the
bottom. If there is a convention feel free to change that around.
> > +(define-public qdirstat
> >
> > - (package
> > - (name "qdirstat")
> > - (version "1.6.1")
> > - (source
> > - (origin
> > - (method git-fetch)
> > - (uri (git-reference
> > - (url "https://github.com/shundhammer/qdirstat.git")
> > - (commit version)))
> > - (file-name (git-file-name name version))
> > - (sha256
> > - (base32 "0q77a347qv1aka6sni6l03zh5jzyy9s74aygg554r73g01kxczpb"))))
> > - (build-system gnu-build-system)
> > - (arguments
> > - `(#:phases
> > - (modify-phases %standard-phases
> > - (replace 'configure
> > - (lambda* (#:key outputs #:allow-other-keys)
> > - (invoke "qmake"
> > - (string-append "PREFIX="
> > - (assoc-ref outputs "out"))
> > - (string-append "INSTALL_PREFIX="
> > - (assoc-ref outputs "out"))))))))
>
> It is our custom to have all phases return #t
I don't really know what this means, looked through the documentation but all I can workout is that something needs to pass what I assume is 'true' to somewhere else. I took this verbatim from another definition in guix.
> > - (native-inputs
> > - `(("zlib" ,zlib)
>
> I think zlib should be an input rather than a native-input since the code
> seems to link to it. Remember that the reason for native-inputs verses
> inputs is that native-inputs are needed on the host side when cross
> compiling.
Done.
> > - (home-page "https://github.com/shundhammer/qdirstat")
> > - (synopsis "Graphical disk space inspection utility")
> > - (description
> > - "QDirStat is a graphical application to show where your disk space has
> > +gone and to help you to clean it up. Shaded boxes represent files and files
> > +are grouped by directory structure." )
>
> Some formatting nitpicks: you can go ahead and start the description on
> the same line as "(description".
Done, though I think it looks better to have it all columnised if it needs to be
broken up at 78chars. Also makes M-q in emacs behave itself.
> Also there is an extra space between the closing quote and closing parentheses.
Blaming this on my tools as well possibly even the linter but I cant exactly recall what did it.
> The best terms I found when searching for similar packages that we already
> have in Guix where, "disk usage", so I think it would be good idea to work
> usage into the synopsis or description somehow. Perhaps change the
> synopsis to say, "disk usage inspection" or the description to say, "show
> your disk usage and help you clean it up."
Done and added analys- root word in case that comes to someone's mind rather
than inspection.
> > - (license license:gpl2+)))
>
> qdirstat-cache-writer has a different license in its header. It's an …
> interesting licence :) I think it's free software, so eligable for
> inclusion in Guix, but probably worth recording here, so that the licence
> field becomes:
>
> (license (list license:gpl2+
> license:non-copyleft)) ; scripts/qdirstat-cache-writer
Done.
> For bonus points, it might be nice to move qdirstat-cache-writer to its
> own output since it is made to be run independently of QDirStat and that
> way it could be installed without pulling in all the C++ and Graphical
> dependencies.
How do you think this should look? Would I make another output in this
qdirstat.scm?
> > -
> > - (inputs
> > - `(("qtbase" ,qtbase)))
> >
> >
>
> I think that perl should be added as input, so that the #! line of
> qdirstat-cache-writer can be patched to refer to a perl in the store.
I'm leaving this for once I've moved qdirstat-cache-writer into a
separate package/output since this definition works for the essential
functionality.
> Can you send an updated patch?
Here and done!
```
From f234ad07e06a5e97fdf1cad36e99e180ae6001f3 Mon Sep 17 00:00:00 2001
From: Thovthe <thovthe@protonmail.com>
Date: Thu, 4 Jun 2020 06:42:54 +0000
Subject: [PATCH] gnu: Add qdirstat.
* gnu/packages/qdirstat.scm: Modified according to feedback
* gnu/local.mk (GNU_SYSTEM_MODULES): Add it.
---
gnu/local.mk | 1 +
gnu/packages/qdirstat.scm | 24 ++++++++++++------------
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/gnu/local.mk b/gnu/local.mk
index 3c9a10b6bc..d1adcbeaa2 100644
--- a/gnu/local.mk
+++ b/gnu/local.mk
@@ -440,6 +440,7 @@ GNU_SYSTEM_MODULES = \
%D%/packages/toys.scm \
%D%/packages/tryton.scm \
%D%/packages/qt.scm \
+ %D%/packages/qdirstat.scm \
%D%/packages/radio.scm \
%D%/packages/ragel.scm \
%D%/packages/rails.scm \
diff --git a/gnu/packages/qdirstat.scm b/gnu/packages/qdirstat.scm
index 91581a206e..a450b8a51a 100644
--- a/gnu/packages/qdirstat.scm
+++ b/gnu/packages/qdirstat.scm
@@ -1,10 +1,10 @@
-(define-module (qdirstat)
- #:use-module (gnu packages qt)
+(define-module (gnu packages qdirstat)
#:use-module (gnu packages compression)
+ #:use-module (gnu packages qt)
#:use-module (guix build-system gnu)
#:use-module (guix git-download)
- #:use-module ((guix licenses) #:prefix license:)
- #:use-module (guix packages))
+ #:use-module (guix packages)
+ #:use-module ((guix licenses) #:prefix license:))
(define-public qdirstat
@@ -33,14 +33,14 @@
(assoc-ref outputs "out"))))))))
(inputs
- `(("qtbase" ,qtbase)))
+ `(("qtbase" ,qtbase)
+ ("zlib" ,zlib)))
(native-inputs
- `(("zlib" ,zlib)
- ("qttools" ,qttools)))
+ `(("qttools" ,qttools)))
(home-page "https://github.com/shundhammer/qdirstat")
(synopsis "Graphical disk space inspection utility")
- (description
- "QDirStat is a graphical application to show where your disk space has
-gone and to help you to clean it up. Shaded boxes represent files and files
-are grouped by directory structure." )
- (license license:gpl2+)))
+ (description "QDirStat is a graphical application for analysing disk usage. It shows
+where your disk space has gone and helps you clean it up. Shaded boxes
+represent files and files are grouped by directory structure.")
+ (license (list license:gpl2+
+ license:non-copyleft)))) ; scripts/qdirstat-cache-writer
--
2.26.2
```
next prev parent reply other threads:[~2020-06-04 7:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-03 0:28 [bug#41040] [PATCH] Package Definition for QDirStat Thovthe via Guix-patches via
2020-05-05 4:01 ` Jack Hill
2020-05-14 0:36 ` Jack Hill
2020-06-04 7:40 ` Thovthe via Guix-patches via [this message]
2020-06-07 4:23 ` Jack Hill
2020-06-13 21:51 ` Thovthe via Guix-patches via
2020-06-28 12:56 ` Jakub Kądziołka
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='Nc6dd4WaVM3RhEkgX7S588RvUDKStBk52aTYK0lPdSpgZDRY7FVK-PxNfNVy8OUaKJcYe9eYszXFCIOS7ppMQvaVdyVxUy-ByiHoCjSTsf0=@protonmail.com' \
--to=guix-patches@gnu.org \
--cc=41040@debbugs.gnu.org \
--cc=Thovthe@protonmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.