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=611328 Mohammed Safwat <Mohammed_ElAfifi@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Mohammed_ElAfifi@xxxxxxxxx --- Comment #3 from Mohammed Safwat <Mohammed_ElAfifi@xxxxxxxxx> 2010-08-07 10:04:28 EDT --- I amn't a sponsor; this's just a casual review. - You can substitute the project name directly in the Source0 field instead of the macro %{name} just to facilitate tracking the URL for reviewers, but it's a matter of personal prefernce anyway. - Unless you really have a strong reason, you should refrain from using epochs in the package version/release scheme(as currently present in the changelog section). See http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment. - Consider using %{name} instead of the direct package name in the Patch1 tag and the %prep and %files sections. - Just a small(optional) hint about the patch, you can do without the -p1 switch in %patch macro if you generate the patch from inside hydra-5.7-src directory. See http://fedoraproject.org/wiki/PackageMaintainers/CreatingPackageHowTo#.25prep_section:_.25patch_commands for different methods and tips about generating diffs for patches. - It's OK to start your patch sequence at 1(as in Patch1), but patches typically start from 0 onwards(i.e. Patch0, Patch1, and so on). - As the build instructions for the main package hydra and subpackage hydra-frontend, you should put the packages in the BuildRequires tag of the subpackage into the BuildRequires tags in the main package. This'll also make the BuildRequires tags specific to the subpackage redundant and unnecessary, and should be therefore removed. - The comment above Patch1 should indicate its status with the upstream(or indicating it's fedora specific if it really is). See http://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment for details. - The INSTALL file shouldn't be included in the files installed by the package as %doc, since it contains manual build instructions not relevant to the user. You can remove it under %install section with `rm INSTALL' just before the make install step. - Under %install section, consider using make with INSTALL="install -p" to preserve timestamps. - If the Makefile supports DESTDIR(which is most likely the case), consider using DESTDIR instead of PREFIX and DIR with make install. -- 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. _______________________________________________ package-review mailing list package-review@xxxxxxxxxxxxxxxxxxxxxxx https://admin.fedoraproject.org/mailman/listinfo/package-review