From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ra Subject: Re: [PATCH] Add r-scran 1.2.0 and all its dependencies Date: Tue, 27 Dec 2016 16:34:16 +0000 Message-ID: References: <87k2aljr6j.fsf@elephly.net> Mime-Version: 1.0 Content-Type: multipart/alternative; boundary=047d7b873f80ff24860544a66cd3 Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:51699) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cLuhr-000733-Qk for guix-devel@gnu.org; Tue, 27 Dec 2016 11:34:33 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cLuhq-0003HD-E1 for guix-devel@gnu.org; Tue, 27 Dec 2016 11:34:31 -0500 Received: from mail-wj0-x235.google.com ([2a00:1450:400c:c01::235]:35139) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cLuhq-0003Gs-37 for guix-devel@gnu.org; Tue, 27 Dec 2016 11:34:30 -0500 Received: by mail-wj0-x235.google.com with SMTP id v7so321950649wjy.2 for ; Tue, 27 Dec 2016 08:34:28 -0800 (PST) In-Reply-To: <87k2aljr6j.fsf@elephly.net> List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane.org@gnu.org Sender: "Guix-devel" To: Ricardo Wurmus Cc: guix-devel --047d7b873f80ff24860544a66cd3 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Hi Ricardo, On Tue, Dec 27, 2016 at 5:02 PM Ricardo Wurmus wrote: Hi Raoul, > in attachment a patch for the inclusion of an R/Bioconductor library named > "scran" https://bioconductor.org/packages/release/bioc/html/scran.html , > scran has a lot of dependencies and I placed those related to > bioinformatics/bioconductor into bioinformatics.scm and those related jus= t > to R insto statistics.scm. Nice! Thank you for your efforts! Thanks, the result worth the effort. > Let me know if this big-patch is acceptable or how do I need to submit it= . We have a policy of one commit per package. Each commit has a commit summary in a standard format (you=E2=80=99ll see when running git log). Ok, when I was adding r-scran I was just trying the make it the work. I'll go back and create a patch for each dependency. Going through the big patch I noticed a couple of things: * the indentation is rather uneven. If you=E2=80=99re using Emacs it=E2=80= =99s very simple to correct this by using TAB and/or hitting C-M-q on the expression. (Also make sure parentheses are not all by themselves on an otherwise empty line.) I was using Vim, I will try with Emacs and also I was practicing a bit. * line length (especially in descriptions) exceeds the limits. * Please edit the descriptions. Package descriptions for Guix have to use full sentences. Judging from the indentation this doesn=E2=80=99t look like it was created = by the *recursive* importer. Were there any problems using the recursive CRAN/Bioconductor importer? with guix 0.12.0 I was able to use guix import, I had some problem in the past due to certificates and x509. I used: guix import cran --archive=3Dbioconductor scater I had to add the missing dependencies by hand, is there a recursive way to do so? The package for =E2=80=9Cr-shiny=E2=80=9D needs a closer look. AFAIR it co= ntains minified Javascript, which does not count as actual source code. If that=E2=80=99s in fact the case we would need to fiddle with the package a little to replace minified Javascript blobs with Javascript source files (which may be minified as part of the build process). I=E2=80=99d prefer i= f =E2=80=9Cr-shiny=E2=80=9D were added to =E2=80=9Cweb.scm=E2=80=9D. Ok for web.scm. In the =E2=80=9Cr-statmod=E2=80=9D package expression the comment above the= gfortran input (which should be a native input) doesn=E2=80=99t seem to apply. Ok, bad comment placed there during tests. Please also double-check that the licenses are correct. The importer may not necessary get things right. Actually I noticed that and I fixed them, I will check again. Finally, please use lower-case words in the synopsis. In R packages this is often capitalised. Ok. Could you please send an updated patch set? Sure, in a couple of days, I think. -- Raoul --047d7b873f80ff24860544a66cd3 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Ricardo,

On Tue, Dec 27, 2016 at 5:= 02 PM Ricardo Wurmus <rekado@elephly.net> wrote:

Hi Raoul,

> in attachment a patch for the inclusion of an=C2=A0 R/Bioconductor lib= rary named
> "scran" https://bioconductor.org/packages/release/bioc/html/scran.html , > scran has a lot of dependencies and I placed those related to
> bioinformatics/bioconductor into bioinformatics.scm and those related = just
> to R insto statistics.scm.

Nice!=C2=A0 Thank you for your efforts!
Thanks, the result worth the effort.
=C2=A0

> Let me know if this big-patch is acceptable or how do I need to submit= it.

We have a policy of one commit per package.=C2=A0 Each commit has a commit<= br class=3D"gmail_msg"> summary in a standard format (you=E2=80=99ll see when running git log).
Ok, wh= en I was adding r-scran I was just trying the make it the work. I'll go= back and create a patch for each dependency.


Going through the big patch I noticed a couple of things:

* the indentation is rather uneven.=C2=A0 If you=E2=80=99re using Emacs it= =E2=80=99s very
=C2=A0 simple to correct this by using TAB and/or hitting C-M-q on the
=C2=A0 expression.=C2=A0 (Also make sure parentheses are not all by themsel= ves on
=C2=A0 an otherwise empty line.)
I= was using Vim, I will try with Emacs and also I was practicing a bit.
=C2=A0

* line length (especially in descriptions) exceeds the limits.

* Please edit the descriptions.=C2=A0 Package descriptions for Guix have to=
=C2=A0 use full sentences.

Judging from the indentation this doesn=E2=80=99t look like it was created = by
the *recursive* importer.=C2=A0 Were there any problems using the recursive=
CRAN/Bioconductor importer?
with g= uix 0.12.0 I was able to use guix import, I had some problem in the past du= e to certificates and x509.
I used:

guix import cran --archive=3Dbioconductor scater
<= /div>

I had to add the missing= dependencies by hand, is there a recursive way to do so?



The package for =E2=80=9Cr-shiny=E2=80=9D needs a closer look.=C2=A0 AFAIR = it contains
minified Javascript, which does not count as actual source code.=C2=A0 If that=E2=80=99s in fact the case we would need to fiddle with the package a<= br class=3D"gmail_msg"> little to replace minified Javascript blobs with Javascript source files (which may be minified as part of the build process).=C2=A0 I=E2=80=99d pre= fer if
=E2=80=9Cr-shiny=E2=80=9D were added to =E2=80=9Cweb.scm=E2=80=9D.

Ok for web.scm.
= =C2=A0

In the =E2=80=9Cr-statmod=E2=80=9D package expression the comment above the= gfortran
input (which should be a native input) doesn=E2=80=99t seem to apply.
Ok, bad comment placed there during tes= ts.
=C2=A0
Please also double-check that the licenses are correct.=C2=A0 The importer<= br class=3D"gmail_msg"> may not necessary get things right.
Actually I noticed that and I fixed them, I will check again.=C2=A0
=

Finally, please use lower-case words in the synopsis.=C2=A0 In R packages this is often capitalised.
Ok.=C2= =A0

Could you please send an updated patch set?
Sure, in a couple of days, I think.

--
Raoul
--047d7b873f80ff24860544a66cd3--