Bug 472149 - Review Request: refmac-dictionary - chemical ligand dictionaries
Review Request: refmac-dictionary - chemical ligand dictionaries
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jason Tibbitts
Fedora Extras Quality Assurance
:
Depends On:
Blocks: coot
  Show dependency treegraph
 
Reported: 2008-11-18 19:04 EST by Tim Fenn
Modified: 2009-01-13 18:22 EST (History)
4 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-01-13 18:22:53 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
tibbs: fedora‑review+
kevin: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Tim Fenn 2008-11-18 19:04:59 EST
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 04:47:00 EST
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 14:12:44 EST
(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-20 20:55:12 EST
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-21 19:05:41 EST
(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-21 19:31:01 EST
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-21 19:56:36 EST
(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 00:21:25 EST
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-17 21:35:07 EST
(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-17 21:40:11 EST
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-20 23:34:56 EST
cvs done.
Comment 12 Tim Fenn 2008-12-23 21:23:13 EST
commits/tags/builds done, submitted to bodhi as newpackage.
Comment 13 Jason Tibbitts 2009-01-13 18:22:53 EST
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.