[Bug 458338] Review Request: DivFix++ - A program to repair broken AVI file streams by rebuilding index part of file

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





--- Comment #3 from Pavel Alexeev <pahan at hubbitus.spb.su>  2008-10-07 17:33:35 EDT ---
First, Mamoru Tasaka, thank you for the review.

(In reply to comment #2)
> Some notes:
> 
> * Macros
>   - Defining %_prefix is not needed
Ok.
>   - jobs is not defined on Fedora. Also please support
>     parallel make if possible:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#Parallel_make
Ok.
>   - %distname is not defined.
%distname replaced by %{distribution}

> * SourceURL
>   - must be written with full URL. For sourceforge hosted tarball,
>     please refer to
>     https://fedoraproject.org/wiki/Packaging/SourceURL#Sourceforge.net
Ok.

> * BuildRoot
>   - does not honor Fedora packaging guidelines:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#BuildRoot_tag
Set to %{_tmppath}/%{name}-%{version}-%{release}-root-%(%{__id_u} -n)

> * Compilation flags
>   - Fedora specific compilation flags are not correctly honored.
>     You can check what compilation flags are used by
>     $ rpm --eval %optflags
>     https://fedoraproject.org/wiki/Packaging/Guidelines#Compiler_flags
>   - Also compilation flag "-Os" supersedes "-O2" flag in %optflags so
>     please remove this.
Excuse me, but I do ot completely understand you. I'm do ot provide manually
any compilation flags, so, default must be used.

> * Timestamps
>   - When using "cp" or "install" commands, add "-p" option to keep
>     timestamps on installed files.
Ok.

> * Desktop file
>   - When using only basename of the icon file is used for Icon entry
>     removing suffix is preferred:
>     https://fedoraproject.org/wiki/Packaging/Guidelines#Desktop_files
Ok, extension .png removed.
> * %clean
>    - The part "[ -d %{buildroot} -a "%{buildroot}" != "" ] && "
>      is not needed
Ok.
> * Inconsistent usage of %buildroot vs $RPM_BUILD_ROOT
>   - Please choose one and not use both
Ok.
> * %defattr
>   - We recommend to use %defattr(-,root,root,-)
Ok.

http://hubbitus.net.ru/rpm/Fedora9/DivFix++/DivFix++-0.30-1.fc9.src.rpm

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