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=510734 --- Comment #43 from Pavel Alexeev (aka Pahan-Hubbitus) <pahan@xxxxxxxxxxxxx> 2009-08-04 06:38:28 EDT --- (In reply to comment #40) > The spec file should be easily readable without any specific tab width > settings. Please use either a standard tab width or convert it to spaces. You are first reviewer what don't accept that. File is easy readable. This style of formating not covered any guidelines, as I can understand (kick me, if I wrong) and I want leave it as it is. > I'm not aware of any explicit documentation which requires "Source0", however > this is also some kind of standard in all Fedora packages. E.g. see the > examples here: > http://fedoraproject.org/wiki/Packaging/SourceURL Only examples? I'm make decision what this is some kind of maintainer preferables only such as using $RPM_BUILD_ROOT or %{buildroot}, rm or %{__rm} etc... > Since this seems to be discouraged in Fedora, please don't do it. The > guidelines don't explicitly forbid this, but at least for the Requires field it > is discouraged: http://fedoraproject.org/wiki/Packaging/Guidelines#Requires This covered small later: http://fedoraproject.org/wiki/Packaging/Guidelines#File_Dependencies Cite from it: Rpm gives you the ability to depend on files instead of packages. Whenever possible you should avoid file dependencies outside of /etc, /bin, /sbin, /usr/bin, or /usr/sbin. Using file dependencies outside of those directories requires yum (and other depsolvers using the repomd format) to download and parse a large xml file looking for the dependency. Helping the depsolvers avoid this processing by depending on the package instead of the file saves our end users a lot of time. =====/ End cite ==== So, key there "end users". Small later in this guidelines you can also find useful example when it is preferable. Nothing there about BuildRequires! So I don't see what it would be discouraged. > > > Use the macros consistently - one plain "rm" leaked in although you've used > > > "%{__rm}" in all other places... > > Ok. > > There is one missing where your removed the prebuilt clients. Upps. Sorry, I just don't update spec. Now I also replace by macroses other things like mv, sed, ln... > > Some more remarks: > > Would it be possible to link it against the regular liblzo even for the Fedora > package? This would save us one condition. It is possible - http://koji.fedoraproject.org/koji/taskinfo?taskID=1579635 But it some sort of hack. Are you sure what we should do it? > Additional if it would be possible to create a patch which would make this a > compile option to switch between minilzo (which is designed to be internal) and > external lzo then this patch would be hopefully acceptable for upstream. There I agree with you - such option would be appreciated. But it requires some additional times, and I wasn't planing do that. May be in the future. > The part of the %prep section which changes the encoding is not correctly > indented. Hm... What exactly? Everything seems correctly intended for me. http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-7.fc11.src.rpm http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec -- 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. _______________________________________________ Fedora-package-review mailing list Fedora-package-review@xxxxxxxxxx http://www.redhat.com/mailman/listinfo/fedora-package-review