Spec Name or Url: http://rpm.greysector.net/extras/crm114.spec SRPM Name or Url: http://rpm.greysector.net/extras/crm114-0-1.20060118.src.rpm Description: CRM114 is a system to examine incoming e-mail, system log streams, data files or other data streams, and to sort, filter, or alter the incoming files or data streams according to the user's wildest desires. Criteria for categorization of data can be by satisfaction of regexes, by sparse binary polynomial matching with a Bayesian Chain Rule evaluator, or by other means. Note: BuildRequires tre-devel, which I submitted as #187609 .
Taking ownership of this after a LUG member noted it wasn't in Fedora. Review is pending resolution of bug #187609; reporter explained current situation to me via IRC.
If I'm not wrong this bug should block FE-REVIEW and be in ASSIGNED state...
Yes. Jima said on IRC that he'll do it shortly, no worries.
Patrice: Don't worry, Rathann and I are in active communication; he knows exactly what my status on this review is. :-) As for why I hadn't blocked FE-REVIEW yet, I was waiting until its dependent bug (187609) cleared review. There were some issues, but they've, well, sort of been cleared up. Other issues cropped up which delayed my turn in this review, but Rathann was understanding. (Thanks!) Rathann: Off the cuff, since tre has 'ExcludeArch: x86_64', shouldn't crm114 have it, too? I'd also welcome a newer version, if it's available. :-) I'll start poking this a little. Let me know if you have any revisions.
(In reply to comment #4) > Patrice: Don't worry, Rathann and I are in active communication; he knows > exactly what my status on this review is. :-) You both know, but setting FE-REVIEW helps others know :-), so it is important to set it when you are sure that you'll review the package such that it appears as such, for example here: https://bugzilla.redhat.com/bugzilla/showdependencytree.cgi?id=FE-REVIEW&hide_resolved=1 and also don't appear anymore in FE-NEW as a package to be reviewed... not a big deal, though.
Release: %{rel}.%{cvsver} - you need the %{?dist} tag adding %build %{__make} %{?_smp_mflags} INSTALL_DIR=$RPM_BUILD_ROOT%{_bindir} CFLAGS="$RPM_OPT_FLAGS" does the install dir need to be here (and the CFLAGS)? install -d $RPM_BUILD_ROOT{%{_bindir},%{_datadir}/%{name}} the make install should create these directories for you. If they don't, mkdir -p is the way to go. %{__make} BINDIR=${RPM_BUILD_ROOT}%{_bindir} install Isn't make DEST_DIR=%{buildroot} install more usual? %{_datadir}/%{name}/*.crm This just needs to be %{_datadir}/%{name}/ The binary files should already be 755, so the %defattr before them shouldn't be required.
http://rpm.greysector.net/extras/crm114.spec http://rpm.greysector.net/extras/crm114-0-1.20060704a.src.rpm New version, fixed parallel make and rpmlint problems.
Going down my own review checklist: http://beer.tclug.org/fedora-extras/review-checklist.txt 1. One rpmlint error; will cover below. 2. I have some concerns about the version/release of this package. Will discuss below. 3. Spec is named crm114.spec, check. 4. Package meets Packaging Guidelines, AFAICT. 5. Licensed under GPL, check. 6. License: GPL, check. 7. n/a, no LICENSE/COPYING file in tarball. 8. Spec appears to be American English. 9. Spec seems legible. 10. md5sum on crm114-20060704a-BlameRobert.no-TRE.src.tar.gz does NOT match upstream. WTF? 11. Compiles and builds on i386/ppc (my two supported build platforms). 12. x86_64 excluded, as per dependency on tre-devel. You should know the drill. 13. Builds under Plague, so I imagine all of its dependencies are listed. 14. n/a, I think. 15. n/a (no shared libs) 16. n/a 17. crm114-emacs does not own %{_datadir}/emacs/site-lisp/; I believe emacs-el does. Please add a dependency. 18. No duplicate %files entries. 19. Not 100% certain on the defattr for the main package. 20. Has valid %clean section. 21. Macro use appears consistent. 22. Package contains code, not content. 23. The doc directory makes up more than half the package's size; you may want to consider splitting it off to a -doc subpackage. (As discussed on IRC.) 24. I don't see anything in %doc affecting runtime. 25. No header files or static libraries. 26. No .pc files. 27. No library files, much less ones with suffixes. 28. n/a (no -devel subpackage) 29. No .la files. 30. No GUI applications. 31. Doesn't own any directories owned by other packages (to the best of my knowledge). 32. Packager should poke upstream to include a LICENSE file. 33. I'm not sure there are any description/summary translations available. 34. Package builds as i386 and ppc in Plague (and thus Mock). 35. Package won't build on x86_64 due to dependency's ExcludeArch: x86_64; other architectures, yes. 36. I can't verify full functionality, but the binary doesn't segfault on i386/ppc. 37. No scriptlets. 38. The -emacs subpackage doesn't Requires: %{name} = %{version}-%{release}; submitter may want to consider doing so. So the standing issues, as I see them (some of which we discussed), are: - rpmlint returns: W: crm114-emacs no-documentation Not sure it's worth remedying. I welcome more experienced reviewers to chime in on this. - I'm concerned about the versioning scheme. How will you be able to re-release 20060704a if there's a bug? Incrementing Release to 2.20060704a will preclude resetting the first digit to 1 with the next release. Also, I believe the 'a' violates the Naming Guidelines. Again, I welcome outside feedback on this. - #10 concerns me greatly. Did you repack the tarball or something? - I think you're missing ExcludeArch: x86_64 (I wouldn't notice, as I don't have an x86_64 buildsys, but this has been a subject of discussion). - Maybe add Req: emacs-el for -emacs, due to that package owning the directory the file is in. (I believe you did this already, offline.) - Is %defattr(-,root,root,-) well-defined enough? - You might want to split out the documentation to a subpackage. - Maybe ask upstream to provide a LICENSE file. - Maybe add Req in #38. I'm sure we'll be in touch.
Hmm, I guess Firefox's focus was on Component when I hit PageUp. Oops!
http://rpm.greysector.net/extras/crm114.spec http://rpm.greysector.net/extras/crm114-0-0.1.20060704.src.rpm Changed the versioning like we discussed on IRC. Fixed all of the above, too. Added %check. Referenced the bug in tre which prevents x86_64 version from working.
Going down my checklist again... 1. One rpmlint warning (W: crm114-emacs no-documentation), which was deemed acceptable. 2. Package appears to meet Package Naming Guidelines. 3. Spec is named crm114.spec, check. 4. Package meets Packaging Guidelines, AFAICT. 5. Licensed under GPL, check. 6. License: GPL, check. 7. %doc contains GPL-License.txt, which I missed on my first pass. 8. Spec appears to be American English. 9. Spec seems legible. 10. md5sum on tarball matches upstream now (not sure what was up with that). 11. Compiles and builds on i386/ppc (my two supported build platforms). 12. x86_64 excluded, as per dependency on tre-devel. You noted bug #202893, the blocker. Good. 13. Builds under Plague, so I imagine all of its dependencies are listed. 14. n/a, I think. 15. n/a (no shared libs) 16. n/a 17. You changed crm114-emacs' Req to emacs-el, resolving this issue. 18. No duplicate %files entries. 19. Defattr seems valid. 20. Has valid %clean section. 21. Macro use appears consistent. 22. Package contains code, not content. 23. Documentation makes up over 50% of the package's size, but that's still not that much. 24. I don't see anything in %doc affecting runtime. 25. No header files or static libraries. 26. No .pc files. 27. No library files, much less ones with suffixes. 28. n/a (no -devel subpackage) 29. No .la files. 30. No GUI applications. 31. Doesn't own any directories owned by other packages (to the best of my knowledge). 32. n/a, I overlooked GPL-License.txt 33. I'm not sure there are any description/summary translations available. 34. Package builds as i386 and ppc in Plague (and thus Mock). 35. Package won't build on x86_64 due to dependency's ExcludeArch: x86_64; other architectures, yes. 36. I can't verify full functionality, but the binary doesn't segfault on i386/ppc. 37. No scriptlets. 38. The -emacs subpackage doesn't depend on the main package, ergo no listed Req. Unless I screwed something up, it looks like crm114 is APPROVED. Go forth and import. :-)
Imported and built for devel. FC5 branch requested. Should I try and convince FESCo to allow FC4 branch? tre managed to get in...
Package Change Request ====================== Package Name: foobar New Branches: EL-5
Package Change Request ====================== Package Name: crm114 New Branches: EL-5 Of course, I meant this. :)