Bug 472149 - Review Request: refmac-dictionary - chemical ligand dictionaries
Summary: Review Request: refmac-dictionary - chemical ligand dictionaries
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Jason Tibbitts
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: coot
TreeView+ depends on / blocked
 
Reported: 2008-11-19 00:04 UTC by Tim Fenn
Modified: 2009-01-13 23:22 UTC (History)
4 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2009-01-13 23:22:53 UTC
Type: ---
Embargoed:
j: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Tim Fenn 2008-11-19 00:04:59 UTC
Spec URL: http://www.stanford.edu/~fenn/packs/refmac-dictionary.spec
SRPM URL: http://www.stanford.edu/~fenn/packs/refmac-dictionary-5.02-1.f8.src.rpm

Description: The refmac ligand dictionaries contain chemical information on a large number of molecules, including the chemical structure of the ligand,
the tree-like structure, the links between ligands, and possible
modifications to them.  This information is stored in the mmCIF
format, which is used by a number of molecular viewing, refinement and
validation tools.

Also see: http://www.ysbl.york.ac.uk/~garib/refmac/latest_refmac.html

Comment 1 Parag AN(पराग) 2008-11-19 09:47:00 UTC
suggestions
1) rpmlint is NOT silent for SRPM and for RPM.
refmac-dictionary.noarch: W: no-version-in-last-changelog
refmac-dictionary.src: W: no-version-in-last-changelog
==> missing version from changelog. Follow guidelines 
http://fedoraproject.org/wiki/PackagingGuidelines#Changelogs

2) Don't mix macros follow http://fedoraproject.org/wiki/PackagingGuidelines#Macros
either use %{buildroot}  OR $RPM_BUILD_ROOT

Comment 2 Tim Fenn 2008-11-19 19:12:44 UTC
(In reply to comment #1)
> suggestions
> 1) rpmlint is NOT silent for SRPM and for RPM.
> refmac-dictionary.noarch: W: no-version-in-last-changelog
> refmac-dictionary.src: W: no-version-in-last-changelog
> ==> missing version from changelog. Follow guidelines 
> http://fedoraproject.org/wiki/PackagingGuidelines#Changelogs
> 

Oops, fixed.

> 2) Don't mix macros follow
> http://fedoraproject.org/wiki/PackagingGuidelines#Macros
> either use %{buildroot}  OR $RPM_BUILD_ROOT

Fixed.

Spec URL: http://www.stanford.edu/~fenn/packs/refmac-dictionary.spec
SRPM URL:
http://www.stanford.edu/~fenn/packs/refmac-dictionary-5.02-2.f8.src.rpm

Comment 3 Jason Tibbitts 2008-11-21 01:55:12 UTC
Can you indicate where you see a statement that this package is under LGPLv3?  All I see is the COPYING file, which has the usual language indicating that any LGPL version applies unless there is some indication of a specific choice of version.  This would indicate LGPLv2+ (since there was no LGPLv1).

In case you don't know and don't like typing, you can write BuildArchitectures: as just BuildArch:.

Is there any reason to have the "data" directory, instead of just putting the monomers directly into %{_datadir}/%{name}-%{version}?  It just seems a bit odd to have two successive directories with nothing in them except for another directory.

Comment 4 Tim Fenn 2008-11-22 00:05:41 UTC
(In reply to comment #3)
> Can you indicate where you see a statement that this package is under LGPLv3? 
> All I see is the COPYING file, which has the usual language indicating that any
> LGPL version applies unless there is some indication of a specific choice of
> version.  This would indicate LGPLv2+ (since there was no LGPLv1).
> 

Sorry, I assumed LGPLv3 since it says "version 3" at the top of the copying file, and appears to be an exact copy of:

http://www.gnu.org/licenses/lgpl-3.0.txt

> 
> Is there any reason to have the "data" directory, instead of just putting the
> monomers directly into %{_datadir}/%{name}-%{version}?  It just seems a bit odd
> to have two successive directories with nothing in them except for another
> directory.

Right - the only reason for that is most programs that access the data assume the files are under datadir/data/monomers/ - I can simply make a symbolic link instead, though, if that would be more appropriate.

Comment 5 Jason Tibbitts 2008-11-22 00:31:01 UTC
Sure, it says version 3, but it pays to actually read the license.  See section 6:

"
If the Library as you received it does not specify a version number of the GNU Lesser General Public License, you may choose any version of the GNU Lesser General Public License ever published by the Free Software Foundation.
"

Our licensing page also covers this:

"
A GPL or LGPL licensed package that lacks any statement of what version that it's licensed under in the source code/program output/accompanying docs is technically licensed under *any* version of the GPL or LGPL, not just the version in whatever COPYING file they include.
"

http://fedoraproject.org/wiki/Licensing

There's no need for a symlink; if other programs expect to see it there then there's no problem.

Comment 6 Tim Fenn 2008-11-22 00:56:36 UTC
(In reply to comment #5)
> 
> Our licensing page also covers this:
> 
> "
> A GPL or LGPL licensed package that lacks any statement of what version that
> it's licensed under in the source code/program output/accompanying docs is
> technically licensed under *any* version of the GPL or LGPL, not just the
> version in whatever COPYING file they include.
> "
> 
> http://fedoraproject.org/wiki/Licensing
> 

Sorry, I didn't realize this.  Changed to LGPLv2+ for now (and I'll talk with upstream about including a specific statement as to which version they intended).

Spec URL: http://www.stanford.edu/~fenn/packs/refmac-dictionary.spec
SRPM URL:
http://www.stanford.edu/~fenn/packs/refmac-dictionary-5.02-3.f8.src.rpm

Comment 8 Jason Tibbitts 2008-12-17 05:21:25 UTC
The above srpm URL is invalid, but I poked around in the directory and found it.  Sorry for not getting back to this sooner.

Note that the leading blank line in your %description makes it into the final output, which probably isn't what you want.  That's really minor, and I can find nothing worth complaining about.

* source files match upstream.  sha256sum:
  803b7669e09124012db110da71fa8dfdf64ea53c92b4021fbaba3e5638aea00b  
   refmac_dictionary_v5.04.tar.gz
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text included in package.
* latest version is being packaged.
* BuildRequires are proper (none).
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   refmac-dictionary = 5.04-1.fc11
  =
   (nothing)
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* acceptable content.

APPROVED

The package review process needs reviewers!  If you haven't done any package
reviews recently, please consider doing one.

Comment 9 Tim Fenn 2008-12-18 02:35:07 UTC
(In reply to comment #8)
> The above srpm URL is invalid, but I poked around in the directory and found
> it.  Sorry for not getting back to this sooner.
> 

Oops, sorry!

> Note that the leading blank line in your %description makes it into the final
> output, which probably isn't what you want.  That's really minor, and I can
> find nothing worth complaining about.
> 

I fixed it anyway:

Spec URL: http://www.stanford.edu/~fenn/packs/refmac-dictionary.spec
SRPM URL:
http://www.stanford.edu/~fenn/packs/refmac-dictionary-5.04-2.fc10.src.rpm

> 
> The package review process needs reviewers!  If you haven't done any package
> reviews recently, please consider doing one.

I'll try - now that I'm more familiar with the process, I'm a little more confident in knowing what to look for.

Comment 10 Tim Fenn 2008-12-18 02:40:11 UTC
New Package CVS Request
=======================
Package Name: refmac-dictionary
Short Description: chemical ligand dictionary
Owners: timfenn
Branches: F-9 F-10 EL-5
InitialCC: timfenn

Comment 11 Kevin Fenzi 2008-12-21 04:34:56 UTC
cvs done.

Comment 12 Tim Fenn 2008-12-24 02:23:13 UTC
commits/tags/builds done, submitted to bodhi as newpackage.

Comment 13 Jason Tibbitts 2009-01-13 23:22:53 UTC
This seems to have been pushed out now; I'll go ahead and close the ticket.


Note You need to log in before you can comment on or make changes to this bug.