[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add r-scran 1.2.0 and all its dependencies
From: |
Ricardo Wurmus |
Subject: |
Re: [PATCH] Add r-scran 1.2.0 and all its dependencies |
Date: |
Tue, 27 Dec 2016 17:02:44 +0100 |
User-agent: |
mu4e 0.9.16; emacs 25.1.1 |
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 just
> to R insto statistics.scm.
Nice! Thank you for your efforts!
> 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’ll see when running git log).
Going through the big patch I noticed a couple of things:
* the indentation is rather uneven. If you’re using Emacs it’s 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.)
* 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’t look like it was created by
the *recursive* importer. Were there any problems using the recursive
CRAN/Bioconductor importer?
The package for “r-shiny” needs a closer look. AFAIR it contains
minified Javascript, which does not count as actual source code. If
that’s 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’d prefer if
“r-shiny” were added to “web.scm”.
In the “r-statmod” package expression the comment above the gfortran
input (which should be a native input) doesn’t seem to apply.
Please also double-check that the licenses are correct. The importer
may not necessary get things right.
Finally, please use lower-case words in the synopsis. In R packages
this is often capitalised.
Could you please send an updated patch set?
--
Ricardo
GPG: BCA6 89B6 3655 3801 C3C6 2150 197A 5888 235F ACAC
http://elephly.net
- [PATCH] Add r-scran 1.2.0 and all its dependencies, Ra, 2016/12/27
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies,
Ricardo Wurmus <=
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ra, 2016/12/27
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ricardo Wurmus, 2016/12/27
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ra, 2016/12/27
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ra, 2016/12/27
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ra, 2016/12/28
- Re: [PATCH] Add r-scran 1.2.0 and all its dependencies, Ricardo Wurmus, 2016/12/29