[Bug 490723] Review Request: R-IRanges - Low-level containers for storing sets of integer ranges

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


https://bugzilla.redhat.com/show_bug.cgi?id=490723





--- Comment #5 from Pierre-YvesChibon <pingou at pingoured.fr>  2009-03-22 06:05:24 EDT ---

> ! The package license corresponds to the license as mentioned in the
>  DESCRIPTION file and in the website, however several source files
>  contain the statement:
> 
> * This file is copyright 2002 Jim Kent, but license is hereby
> * granted for all use - public, private or commercial. */
> 
>  which is not the same as "Artistic licence 2.0". Needs some
>  consultation with upstream.

I will ask upstream for clarification

> ! Specfile is written in legible English and uses macro consitently,
>  however:
> 
>  The Source and URL fields uses the macro %{BioC} with is not defined
>  anywhere. The urls point to the right location if this macro is
>  assumed to be an empty string, so techncally they are correct, but
>  it is a source of confusion. Suggestion - remove the macros.

The macro is defined (line 2 of the file), however the %define should be
changed to %global according to the newly approved guidelines.

>  The comment that says "#i368 arch" should probaly read something
>  like "#architecture dependent package", because that I think is what
>  you really mean.

That comment is generated by R2spec, it can be changed/ignored/removed
(I will change R2spec to this)

> ! Package compiles, but there are warnings that should be fixed:
> 
> IntervalTree.c:48: warning: missing braces around initializer
> memalloc.c:293: warning: format '%d' expects type 'int', but argument 2 has type 'size_t'
> memalloc.c:293: warning: format '%d' expects type 'int', but argument 3 has type 'size_t'
> 
>  The "... may be used uninitialized" warnings might be harmless (or
>  not, ...)

> I have no idea if these warnings are harmful or not:
> 
> Creating a generic for "cbind" in package  "IRanges"
>    (the supplied definition differs from and overrides the implicit generic in package "base": Signatures differ:  (...), (deparse.level))
> Creating a generic for "rbind" in package  "IRanges"
>    (the supplied definition differs from and overrides the implicit generic in package "base": Signatures differ:  (...), (deparse.level))

I will point these warning to upstream.

> ! %check is present and check pass, but
> 
>  Comparing 'runalltests.Rout' to 'runalltests.Rout.save' ...11,12c11
> < 	 rbind,
> < 	 sapply 
> ---
> > 	 rbind 
> 
>  It test still says "OK", but it looks "strange".

This is just the output of the test run

> ! No package owns the package's main directory /usr/lib64/R/library/IRanges

Oups, I will change this

>  Ideally both main and devel should own the main directory in order
>  to avoid orphaned directories after package removal, but at least
>  main must own it.

I do not agree. Since -devel has the main package has a requirement, only the
main need to own the directory.

> ! Since the -devel package does not include a .pc file it should not
>  require pkgconfig (or is there some other reason for this?)

Will be fixed

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.