Bug 172900
Summary: | Review Request: htop | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Dawid Gajownik <gajownik> |
Component: | Package Review | Assignee: | Dmitry Butskoy <dmitry> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-extras-list |
Target Milestone: | --- | ||
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://htop.sourceforge.net/ | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-11-11 17:37:25 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779 |
Description
Dawid Gajownik
2005-11-10 22:44:37 UTC
Some remarks & nitpicks: - CFLAGS in make line is not needed (it is properly written into Makefile by configure script). - remove AUTHORS (more useful info is already in README) and NEWS (unusable) from the %doc - change "It aims to be a better 'top'" in the description to ", similar to top(1)" . "Aims to be a better" is obvious here (else why add to FE? :)). "top(1)" points the reader that it is some system command "top", rather than something else. rpmlint OK name OK license OK package OK source matches upstream Works fine on the both Linux console and xterm. (In reply to comment #1) > - CFLAGS in make line is not needed (it is properly written into Makefile by > configure script). Ooops. At first I was compiling it by hand I must have used system account without exported CFLAGS variable :/ > - remove AUTHORS (more useful info is already in README) and NEWS (unusable) > from the %doc Done. > - change "It aims to be a better 'top'" in the description to ", similar to > top(1)" I copied this description from project site รข http://sourceforge.net/projects/htop ;-) Updated package is available here: http://wiki.fedora.pl/gajownik/htop/htop.spec http://wiki.fedora.pl/gajownik/htop/htop-0.5.4-2.src.rpm Thanks for the review! OK APPROVED! BTW, optional, the first word in the %description (i.e. "htop") can be changed to %{name} macro... (In reply to comment #4) > BTW, optional, the first word in the %description (i.e. "htop") can be changed > to %{name} macro... That would IMHO be a regression, because it would make the spec file less clear. Macros are useful where their expansions may be expected to change at some point, but that's not the case for the %{name} macro, so I'd try to avoid it in all but the most generic of cases, such as in the standard buildroot [%{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)]. > Macros are useful where their expansions may be expected to change at some
> point, but that's not the case for the %{name} macro,
Why?
If you want to change the name of package (i.e. "Name: bar" instead of "Name:
foo") sometimes in the future, you will must do a lot of manual changes in the
spec file. To avoid this, %{name} macro is used.
IMHO, %{name} is even more clear than original name, because it specifies that
there is PACKAGE NAME in this place, and it is not a word which just matches
with package's name occasionally.
(In reply to comment #6) > > Macros are useful where their expansions may be expected to change at some > > point, but that's not the case for the %{name} macro, > Why? > > If you want to change the name of package (i.e. "Name: bar" instead of "Name: > foo") sometimes in the future, you will must do a lot of manual changes in the > spec file. To avoid this, %{name} macro is used. Seriously, how often do package names change? And if it did, there would probably be a whole host of other changes needed too, such as for the URL, the binary name, the manpage etc. > how often do package names change? Yep, but: > there is PACKAGE NAME in this place, and it is not a word which just matches with package's name occasionally. is a main reason for this. (IMHO). > the binary name, the manpage etc. Therefore people use %{_bindir}/* , %{_mandir}/*/* , %{_initrddir}/* etc. It is not just my opinion, I see such approach at least in half of packages which I looked in... (In reply to comment #8) > > how often do package names change? > Yep, but: > > there is PACKAGE NAME in this place, and it is not a word which just matches > with package's name occasionally. > is a main reason for this. (IMHO). > > > the binary name, the manpage etc. > Therefore people use %{_bindir}/* , %{_mandir}/*/* , %{_initrddir}/* etc. > > It is not just my opinion, I see such approach at least in half of packages > which I looked in... This is also an unfortunate choice IMHO. Such vague wildcarding can lead to problems where a later version of a package from upstream may include files with generic names that could clash with other packages. This is particularly an issue with include files. A packager using wildcards as suggested above might not notice this when updating the package, but a packager that enumerated the files list more specifically would do (they would get an unpackaged file error at rpm build time), and could exclude or rename the offending file as was appropriate. > Such vague wildcarding can lead to
> problems where a later version of a package from upstream may include files
> with generic names that could clash with other packages.
"how often do it change?" ;)
IMHO, a case with file clashing is more rare rather than a necessity to chew a
lot of files at the upstream version change.
Anyway, it is some wide-spread practice, permitted in Fedora. Let the user
choose it, if he(she) prefers this way.
If you have powerful arguments against such way of writing of the spec files,
let's open new thread in fedora-extras list to discuss possible changes for
PackagingGuidelines.
(In reply to comment #4) > APPROVED! Thanks! Package has built correctly so I'm closing this bugreport :] > BTW, optional, the first word in the %description (i.e. "htop") can be changed > to %{name} macro... Well, I left it as is ;) |