Red Hat Bugzilla – Bug 510865
Review Request: ccze - A robust log colorizer
Last modified: 2009-08-26 19:34:33 EDT
Spec URL: http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze.spec SRPM URL: http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze-0.2.1-3.fc11.src.rpm Description: CCZE is a roboust and modular log colorizer, with plugins for apm, exim, fetchmail, httpd, postfix, procmail, squid, syslog, ulogd, vsftpd, xferlog and more. http://koji.fedoraproject.org/koji/taskinfo?taskID=1467983
Doesn't the makefile support DESTDIR? https://fedoraproject.org/wiki/Packaging/Guidelines#Why_the_.25makeinstall_macro_should_not_be_used
Jussi Lehtola, you are right - http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze.spec
http://bonehunter.rulez.org/CCZE.html Not Found The requested URL /CCZE.html was not found on this server.
Upstream website dead many time. Is it really problem? You can see it in archive - link in comment in spec file.
I'd like to do the (informal) review, but the above linked SRPM is not available. Also, the URL of the source tarball given in the spec file is dead. Due to the lack of the sources, I'm unable to build the package. Concerning the spec file, you should correct BuildRoot. According to https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag it must contain name, version and revision.
It seems the SRPM url is now http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze-0.2.1-4.fc11.src.rpm
rpmlint SRPMS/ccze-0.2.1-4.fc11.src.rpm 1 packages and 0 specfiles checked; 0 errors, 0 warnings. rpmlint RPMS/i586/ccze-* 2 packages and 0 specfiles checked; 0 errors, 0 warnings. + package name: ok + spec file name: ok + valid license: ok + license tag: ok - source tarball not available upstream - BuildRoot doesn't comply with packaging guidelines (see comment #5) + BuildRequire tags seem to be complete - you should add %{?_smp_mflags} to the make statement + no locales available + no duplicate files listed + files section and permissions: ok + package owns its files + no development files (headers, libraries) + no large documentation + no sub-packages + no pkgconfig files + install section: ok + no scriptlets
I'm going to sponsor Martin, doing full review.
Martin Gieseking, thank you. All fixed. http://koji.fedoraproject.org/koji/taskinfo?taskID=1579720 http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze-0.2.1-5.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/ccze/ccze.spec
rpmlint output is clean. MUST: The package does not yet exist in Fedora. The Review Request is not a duplicate. OK MUST: The spec file for the package is legible and macros are used consistently. NEEDSWORK - If you use tabbing, do it properly: remove the extra space from Summary, Version, Release and so on. - There is no comment on what patch0 does. - Fix the buildroot as per comment #5. MUST: The package must be named according to the Package Naming Guidelines. OK MUST: The spec file name must match the base package %{name}. OK MUST: The package must be licensed with a Fedora approved license and meet the Licensing Guidelines. OK MUST: The License field in the package spec file must match the actual license. OK MUST: The sources used to build the package must match the upstream source, as provided in the spec URL. ?? - Upstream website is down. MUST: The package MUST successfully compile and build into binary rpms. OK MUST: The spec file MUST handle locales properly. N/A MUST: Optflags are used and time stamps preserved. OK - Man page is generated, so no need to preserve time stamp in install. - Header file time stamp should be preserved, but it is not included in the tarball. MUST: Packages containing shared library files must call ldconfig. N/A MUST: A package must own all directories that it creates or require the package that owns the directory. OK MUST: Files only listed once in %files listings. OK MUST: Debuginfo package is complete. OK MUST: Permissions on files must be set properly. OK MUST: Clean section exists. OK MUST: Large documentation files must go in a -doc subpackage. N/A MUST: All relevant items are included in %doc. Items in %doc do not affect runtime of application. OK MUST: Header files must be in a -devel package. N/A MUST: Static libraries must be in a -static package. N/A MUST: Packages containing pkgconfig(.pc) files must 'Requires: pkgconfig'. N/A MUST: If a package contains library files with a suffix then library files ending in .so must go in a -devel package. N/A MUST: In the vast majority of cases, devel packages must require the base package using a fully versioned dependency. N/A MUST: Packages does not contain any .la libtool archives. N/A MUST: Desktop files are installed properly. N/A MUST: No file conflicts with other packages and no general names. OK MUST: Buildroot cleaned before install. OK SHOULD: %{?dist} tag is used in release. OK SHOULD: If the package does not include license text(s) as separate files from upstream, the packager should query upstream to include it. OK SHOULD: The package builds in mock. OK
(In reply to comment #10) > MUST: The package does not yet exist in Fedora. The Review Request is not a > duplicate. OK > MUST: The spec file for the package is legible and macros are used > consistently. NEEDSWORK > - If you use tabbing, do it properly: remove the extra space from Summary, > Version, Release and so on. There only tabs. On case mixing it, rpmlint comply it. > - There is no comment on what patch0 does. Ok. I comment it. > - Fix the buildroot as per comment #5. It was already done. > MUST: The sources used to build the package must match the upstream source, as > provided in the spec URL. ?? > - Upstream website is down. I known. :( What you can suggest do with that? Must I "fork" project to continue?
(In reply to comment #11) > (In reply to comment #10) > > MUST: The package does not yet exist in Fedora. The Review Request is not a > > duplicate. OK > > MUST: The spec file for the package is legible and macros are used > > consistently. NEEDSWORK > > - If you use tabbing, do it properly: remove the extra space from Summary, > > Version, Release and so on. > There only tabs. On case mixing it, rpmlint comply it. That was a general expression - remove the extra tabs. > > MUST: The sources used to build the package must match the upstream source, as > > provided in the spec URL. ?? > > - Upstream website is down. > I known. :( > What you can suggest do with that? Must I "fork" project to continue? I guess we just have to live with it.
(In reply to comment #12) > That was a general expression - remove the extra tabs. There is not extra tabs! I think this misunderstood because I use tab with in 5 space (this is cose t easy distinguish tabs and space aligments).
(In reply to comment #13) > (In reply to comment #12) > > That was a general expression - remove the extra tabs. > There is not extra tabs! I think this misunderstood because I use tab with in 5 > space (this is cose t easy distinguish tabs and space aligments). Summary: A robust log colorizer Summary(ru): Мощный коллоризатор логов Name: ccze Version: 0.2.1 Release: 5%{?dist} # http://web.archive.org/web/20040803024236/bonehunter.rulez.org/CCZE.phtml URL: http://bonehunter.rulez.org/CCZE.html License: GPLv2+ Group: Applications/Text This is ugly. What it should look like is Summary: A robust log colorizer Summary(ru): Мощный коллоризатор логов Name: ccze Version: 0.2.1 Release: 5%{?dist} # http://web.archive.org/web/20040803024236/bonehunter.rulez.org/CCZE.phtml URL: http://bonehunter.rulez.org/CCZE.html License: GPLv2+ You have one tab too much in some lines.
>You have one tab too much in some lines. Again - ONLY if as you, expand tab to 8 spaces! If you expand its in 5, all indentation seems ok. :)
(In reply to comment #15) > >You have one tab too much in some lines. > Again - ONLY if as you, expand tab to 8 spaces! If you expand its in 5, all > indentation seems ok. :) The tab '\t' is not expanded to ' 's. The spec file is messy in Firefox, Chromium, vim, emacs and nano. Just remove the extra tabs, OK?
Created attachment 356180 [details] Kwrite with tab with=5 space There are NO EXTRA TABS! Please see screenshot in Kwrite and console mcedit. What tabs there "extra"??
Created attachment 356181 [details] Mcedit screenshot
Okay. I tested with Chromium, Firefox, Emacs, Vim and Nano and all of them displayed the spec file otherwise. 8 character tabbing is the standard, and you should use it. ** The package has been APPROVED.
(In reply to comment #19) > 8 character tabbing is the standard Off course >, and you should use it. but not must ;) > The package has been > > APPROVED. Thank you very much for review!
New Package CVS Request ======================= Package Name: ccze Short Description: A robust log colorizer Owners: hubbitus Branches: F-10 F-11 EL-5 InitialCC:
CVS done.
(In reply to comment #19) > 8 character tabbing is the standard, You are in error. 8 char tabs are pretty common, but that's all. > and you should use it. You've got a learning curve ahead - Better concentrate on technical issues and not on cosmetic issues in reviews.
(In reply to comment #23) > > and you should use it. > You've got a learning curve ahead - Better concentrate on technical issues and > not on cosmetic issues in reviews. Yes, this was a minor cosmetic issue (would have been easy to fix too). I've worked on 130 package reviews so far, and this has been the only one using a different tab width. Having two incompatible conventions is really not very useful. Converting the tabs to spaces might be a good option, since that way the tabbing is always correct.
(In reply to comment #24) > (In reply to comment #23) > > > and you should use it. > > You've got a learning curve ahead - Better concentrate on technical issues and > > not on cosmetic issues in reviews. > > Yes, this was a minor cosmetic issue (would have been easy to fix too). I've > worked on 130 package reviews so far, and this has been the only one using a > different tab width. Again, tab widths are personal preference, you can agree with or not. That said, your personal preference is irrelevant. Also, what might be news to apparent newcomers with only little programming experience like you: There exist large communities of programmers who are using other tab widths as their standard (e.g. FreeBSD uses 4, many people with vi-background use 3, etc.) > Having two incompatible conventions is really not very useful. Cosmetics are irrelevant. > Converting the > tabs to spaces might be a good option, since that way the tabbing is always > correct. That's a newbie's bikeshed.
Ralf Corsepius, thank you for the support...
ccze-0.2.1-5.el5 has been submitted as an update for Fedora EPEL 5. http://admin.fedoraproject.org/updates/ccze-0.2.1-5.el5
ccze-0.2.1-5.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/ccze-0.2.1-5.fc10
ccze-0.2.1-5.fc11 has been submitted as an update for Fedora 11. http://admin.fedoraproject.org/updates/ccze-0.2.1-5.fc11
ccze-0.2.1-5.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
ccze-0.2.1-5.fc11 has been pushed to the Fedora 11 stable repository. If problems still persist, please make note of it in this bug report.
ccze-0.2.1-5.el5 has been pushed to the Fedora EPEL 5 stable repository. If problems still persist, please make note of it in this bug report.