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=610073 Mohammed Safwat <Mohammed_ElAfifi@xxxxxxxxx> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |Mohammed_ElAfifi@xxxxxxxxx --- Comment #3 from Mohammed Safwat <Mohammed_ElAfifi@xxxxxxxxx> 2010-08-06 19:43:12 EDT --- I amn't a sponsor; this's just a casual review. - The Version field should reflect the corresponding software version as specified on the project website. I can't find a version matching 20100629 at http://code.google.com/p/flyback/downloads/list. - There's neither a separate license(COPYING) file nor a license header notice in the source files. The specified version, GPLv2, correctly matches the one specified at the project page http://code.google.com/p/flyback/; you should notify the upstream to include a license file and a license notice in the source file headers. This isn't a blocker, however. - The Source0 URL expands to path pertaining to the packager personal space at fedorapeople; this can't be a valid permanent link to get the original software source from. You should specify a download link as provided by the upstream, usually at the software site. - 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. - You should uncomment the BuildRequires filed, stating appropriate required python runtime development libraries(python 2 or python 3). See http://fedoraproject.org/wiki/Packaging:Python#BuildRequires for details. You should also add desktop-file-utils in the BuildRequires field as explained at http://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage, since you've a desktop file in your package. - Under %build section, the instructions section are intended to add a new wrapper shell script to a python script. You should instead create a patch containing this wrapper script and use %patch under %prep section to apply the patch. - Under %install section, you should use the install command with appropriate command-line switches instead of mkdir and cp commands. Consult the install manual as well as http://fedoraproject.org/wiki/Packaging:Python#Byte_compiling for available install options and examples. - Under %install section, to install the desktop file use the command desktop-file-install instead of cp. See http://fedoraproject.org/wiki/Packaging/Guidelines#desktop-file-install_usage for more usage scenarios of this command. - Under %install and %files sections, you should make use of the predefined path macros instead of specifying explicit paths, for example %{_datarootdir} instead of /usr/share, %{_bindir} instead of /usr/bin, and %{_desktopdir} instead of /usr/share/applications, ...etc. The command `rpm --showrc' can help you identify the paths predefined by macros. Check http://fedoraproject.org/wiki/Docs/Drafts/BuildingPackagesGuide#Case_Study:_leafpad for other useful examples. - In the desktop file flyback.desktop, it's better to specify the icon with a short name, but the full path is also OK. See http://fedoraproject.org/wiki/Packaging/Guidelines#Icon_tag_in_Desktop_Files for explanation. - The desktop file contains a deprecated key, FilePattern, as described at http://standards.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#deprecated-items. It should be removed. -- 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