[Bug 653682] Review Request: jemalloc - General-purpose scalable concurrent malloc(3) implementation

[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=653682

Golo Fuchert <packages at golotop.de> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |packages at golotop.de

--- Comment #2 from Golo Fuchert <packages at golotop.de> 2010-11-17 18:30:28 EST ---
Since I am not yet sponsored I can only do an informal review for this package.
Please note that since I am quite new here I may have overseen some details.

---

Informal review:

[+] = ok
[o] = does not apply
[-] = not ok


[+] The rpmlint output seems to be ok:

jemalloc.src: W: spelling-error Summary(en_US) malloc -> mallow, Mallorca,
Mallory
jemalloc.src: W: spelling-error %description -l en_US malloc -> mallow,
Mallorca, Mallory
jemalloc.x86_64: W: spelling-error Summary(en_US) malloc -> mallow, Mallorca,
Mallory
jemalloc.x86_64: W: spelling-error %description -l en_US malloc -> mallow,
Mallorca, Mallory

- I vote for Mallorca, but if you really want to you can stick to malloc. ;-)

jemalloc.x86_64: W: manual-page-warning /usr/share/man/man3/jemalloc.3.gz 65:
warning: `UR' not defined
jemalloc.x86_64: W: manual-page-warning /usr/share/man/man3/jemalloc.3.gz 67:
warning: `UE' not defined

- honestly, I have no idea what those warnings mean. I saw they were no
  blockers in other reviews, but this is a bit disappointing. Maybe someone
  can say more about these.

jemalloc.x86_64: W: no-manual-page-for-binary pprof

- would be nice to have one but no blocker.

2 packages and 1 specfiles checked; 0 errors, 7 warnings.

[+] The package is named according to the guidelines
[+] Spec file name matches base package name
[+] The package follows the Packaging Guidelines
[+] The license is an approved license (BSD)
[+] The License field matches the actual license
[+] License file from source file is included in %doc
[+] The spec file is written in American English
[+] The spec file is legible
[+] Used sources match width upstream sources (md5)
 rpmdev-md5 jemalloc-2.0.1-1.fc14.src.rpm 
 4687f59c073975f39375ece49c402bdc  jemalloc-2.0.1.tar.bz2
 md5sum jemalloc-2.0.1.tar.bz2 
 4687f59c073975f39375ece49c402bdc  jemalloc-2.0.1.tar.bz2
[+] Package build at least on one primary architecture (x86_64 tested)
[+] No architectures are known not to work
[+] All build dependencies are listed in the spec file (checked with mock)
[o] No locales for the package
[+] Package stores shared libraries and calls ldconfig in %post/%postun
[+] Package does not bundle copies of system libraries
[o] Package is not relocatable
[-] Package does not own all directories that are created:
 -devel sub-package creates %{_includedir}/jemalloc but doesn't own it
[+] No files are listed more then once in the %files section
[+] File permissions are set properly (%defattr(...) is used)
[+] Consistent use of macros
[+] Package contains code and documentation only, no content
[o] No large documentation files
[+] %doc files do not affect runtime
[+] No Header files included outside of the -devel package
[o] No static libraries included
[+] Library files ending with .so correctly in a -devel package
[-] -devel package should require the package correctly as %{name} =
%{version}-%{release} (missed the -%{release} part)
[+] No libtool .la archives included
[o] No GUI application
[+] Package does not own files or directories that are owned by other packages
[+] All filenames are valid UTF-8

---

Further comments:
- The description contains a lot of trademarks. As far as I can tell they are
  used properly, but I feel a bit uncomfortable with that. Don't you think
  that the users searching for this kind of package know who uses it for what?
  However, I _think_ it is safe like that but maybe others can comment on this.
- A dot would be the perfect ending to the description of the devel package.
- Extensive globbing in the file section is a dangerous thing; you might
include
  files and dirs you don't want to include and it gets more legible if you are
a
  bit more explicit. Especially when you glob just one file like in
%{_mandir}/man3/*.
- Would do no harm to use the %{name} macro in the SOURCE0 tag.

---

Conclusion:
Yes, the package needs some fixing right now, but please note that I like
the clean look of the SPEC file and overall I think it is almost complete.

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