[Bug 186327] Review Request: tcltls

[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 report.

Summary: Review Request: tcltls


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





------- Additional Comments From wart at kobold.org  2006-05-26 13:26 EST -------
I finally got around to a formal review.  Most of the issues have already been
addreessed above, but there's still a few remaining nits.

MUST
====
* rpmlint output:
  W: tcltls-devel no-documentation
  ...which can be ignored because upstream doesn't provide development
  documentation.
* Source matches upstream
  9eeab472475773b3810acc808ebec759  SOURCES/tls1.5.0-src.tar.gz
* BSD license ok, license file included
* Spec file legible and in Am. English
* No excessive BR:
* No locales
* No shared libraries in the default linker path
* Not relocatable
* Owns the directory that it creates
* No duplicate %files
* buildroot cleaned where needed
* No -docs subpackage needed
* header file included in -devel package
* No .desktop file needed
* Package runs without crashing (tested with tclhttpd)
* Builds in mock on:
  core4-i386, core4-x86_64, core5-i386, core5-x86_64, devel-i386
* File permissions look ok

ANOMOLIES
=========
* Package name does not match upstream (tcltls vs tls).  However, the
  general convention outside of Fedora for naming Tcl packages is "tcl<foo>",
  which is already quite similar to Fedora's naming convention for
  language subpackages "tcl-<foo>".  In this case I feel it's better to
  use either tcl<foo> or tcl-<foo> for the package name as 'tls' is too
  generic.

* The version number is 1.5.0, yet upstream puts files in %{_libdir}/tls1.50.
  You should probably tell upstream to fix their directory name.  You can
  also do it yourself in this package, but I won't consider it a blocker.
  Fixing the directory name would also remove the need for the majorver and
  minorver %defines in the spec file.

MUSTFIX
=======
* A more succinct summary would be "OpenSSL extension for Tcl"
* Summary for the -devel package reads better as:
  "Header files for the OpenSSL extension for Tcl"
* Minor nit: Remove the commented-out entry in %files and in %prep
* Use a macro in %configure:
  %configure --with-ssl-dir=%{_prefix}
* Group should be "Development/Libraries"
* The URL, Version, and Group tags in the -devel subpackage aren't necessary.
  They are inherited from the base package.


-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug, or are watching the QA contact.