GNU bug report logs - #30894
Submission of patch for Add r-abc

Previous Next

Full log

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Sahithi Yarlagadda <sahi <at> swecha.net>
Cc: 30894 <at> debbugs.gnu.org
Subject: Re: [bug#30894] Submission of patch for Add r-abc
Date: Wed, 28 Mar 2018 21:44:26 +0200

Hi Sahiti,

have you been able to look at my review below?
Please let me know if anything is unclear.

~~ Ricardo

Ricardo Wurmus <rekado <at> elephly.net> writes:

> Hi Sahithi,
>
>> I am submitting a patch for r-abc. Please review.
>
> Thank you for the patch!
>
> What follows is a short review that includes things I could change
> myself before applying the patch, but for the sake of learning I
> explicitly point out in this message.  I hope you don’t mind that I’m
> focusing on what may seem like minor details.
>
>> From 4b3b3eee4995616059d8a27489affc43cee26ea4 Mon Sep 17 00:00:00 2001
>> From: Sahithi Yarlagadda <sahi <at> swecha.net>
>> Date: Wed, 21 Mar 2018 16:30:18 +0530
>> Subject: [PATCH] gnu: Add r-abc.
>>
>>     * gnu/packages/cran.scm (r-abc): New variable.
>
> Nitpick: please don’t indent this line.  The asterisk should be the
> first character in this line.
>
>> +
>> +(define-public r-abc
>> +  (package
>> +    (name "r-abc")
>> +    (version "2.1")
>> +    (source
>> +      (origin
>> +        (method url-fetch)
>> +        (uri (cran-uri "abc" version))
>> +        (sha256
>> +          (base32
>> +            "0ngzaaz2y2s03fhngvwipmy4kq38xrmyddaz6a6l858rxvadrlhb"))))
>> +    (build-system r-build-system)
>> +    (propagated-inputs
>> +      `(("r-abc-data" ,r-abc-data)
>> +        ("r-locfit" ,r-locfit)
>> +        ("r-mass" ,r-mass)
>> +        ("r-nnet" ,r-nnet)
>> +        ("r-quantreg" ,r-quantreg)))
>> +    (home-page
>> +      "http://cran.r-project.org/web/packages/abc")
>
> Please pull this onto the previous line and use “https”.
>
>> +    (synopsis
>> +      "Tools for approximate bayesian computation (ABC)")
>
> Please combine this line with the previous line.
>
>> +    (description
>> +      "Implements several ABC algorithms for performing parameter estimation,
>> +model selection, and goodness-of-fit.  Cross-validation tools are also
>> +available for measuring the accuracy of ABC estimates, and to calculate the
>> +misclassification probabilities of different models.")
>
> Please use full sentences.  For CRAN packages the official description
> usually provides only a sentence fragment in place of the first
> sentence.  Please also use “@dfn{Approximate Bayesian Computation}
> (ABC)” the first time “ABC” is mentioned.
>
> All thoughout the indentation is a bit off (that’s expected as the
> importer is very simple and doesn’t know about the expected
> indentation).  If you’re using Emacs you can easily adjust it with M-C-q
> (also known as “indent-sexp”); otherwise please use
> “etc/indent-code.el”.
>
> Have you built this package with Guix?
>
> Finally, please rebase this patch so that it can be applied
> independently from the other patch you’ve sent, which cannot currently
> be applied due to problems in building the defined package.  Rebasing
> this patch ensures that it can be merged even if the other patch is
> stuck.
>
> Thanks!

Previous Next

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