From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:43462) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1fGlLg-00064K-Jw for guix-patches@gnu.org; Thu, 10 May 2018 09:11:09 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1fGlLa-0007kl-H9 for guix-patches@gnu.org; Thu, 10 May 2018 09:11:08 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:48154) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1fGlLa-0007kg-DE for guix-patches@gnu.org; Thu, 10 May 2018 09:11:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1fGlLZ-00029w-Vi for guix-patches@gnu.org; Thu, 10 May 2018 09:11:02 -0400 Subject: [bug#31395] [PATCH 2/2] gnu: Add snap. Resent-Message-ID: From: Nicolas Goaziou In-Reply-To: <20180509213625.5fa4db0a@centurylink.net> (Eric Bavier's message of "Wed, 9 May 2018 21:36:25 -0500") References: <20180509214622.28928-1-mail@nicolasgoaziou.fr> <20180509214622.28928-2-mail@nicolasgoaziou.fr> <20180509213625.5fa4db0a@centurylink.net> Date: Thu, 10 May 2018 15:10:00 +0200 Message-ID: <87y3grpsgn.fsf@nicolasgoaziou.fr> MIME-Version: 1.0 Content-Type: text/plain List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+kyle=kyleam.com@gnu.org Sender: "Guix-patches" To: Eric Bavier Cc: 31395@debbugs.gnu.org Hello, Eric Bavier writes: > This package looks fun, thanks for working on it, I played with it for > a while in my browser :) Heh. > Just a few comments: Thank you for the review. >> + (uri (string-append >> + "https://github.com/jmoenig/Snap--Build-Your-Own-Blocks/archive/" >> + version ".tar.gz")) > > I think we're trying to stay away from Github's auto-generated tarballs > now, because they are not guaranteed to remain the same over time. > > Unfortunately this project doesn't seem to upload its own release > tarballs. I would instead use a git checkout. Version is a tag, therefore a commit, so I would think it should remain identical over time. Besides a number of packages use this (e.g., audacity...) Granted, I'm not well-versed in Github technology. Yet, using a git checkout slightly complicates the package, and its subsequent updates, so I'd rather only use it if absolutely necessary. WDYT? >> + (mkdir-p share) >> + (with-directory-excursion root >> + (copy-recursively "." share)) > > This could be simplified to '(copy-recursively root share)', and the > mkdir-p can even be left out because copy-recursively will create it > for you. OK. >> + (let* ((bin (string-append out "/bin")) >> + (script (string-append bin "/snap")) >> + (bash (string-append (assoc-ref %build-inputs "bash") >> + "/bin/bash")) >> + (xdg-open (string-append (assoc-ref %build-inputs "xdg-utils") >> + "/bin/xdg-open")) >> + (snap (string-append share "/snap.html"))) >> + (mkdir-p bin) >> + (call-with-output-file script >> + (lambda (port) >> + (format port "#!~a\n~a '~a'" bash xdg-open snap))) > > You could maybe use '(which "sh")' and '(which xdg-open)'. A > matter of taste, I think. So (which "sh") should be in the (format ...) call instead of the `bash' binding above? This is shorter, indeed. >> + (native-inputs >> + `(("gzip" ,gzip) >> + ("tar" ,tar) >> + ("js-filesaver" ,js-filesaver))) > > js-filesave should go in "inputs", right? Correct. >> + (inputs >> + `(("bash" ,bash-minimal) >> + ("xdg-utils" ,xdg-utils))) >> + (home-page "https://snap.berkeley.edu") >> + (synopsis "Visual, blocks based programming language inspired by Scratch") > > We can leave out "inspired by Scratch" in the synopsis. OK. >> +This package provides a @command{snap} executable calling @command{xdg-open} >> +to open a the application in a web browser.") > ^ > s/a the/the/ OK. Regards, -- Nicolas Goaziou 0x80A93738