[Bug 610073] Review Request: flyback - time machine for linux

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

Mohammed Safwat <Mohammed_ElAfifi at yahoo.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |Mohammed_ElAfifi at yahoo.com

--- Comment #3 from Mohammed Safwat <Mohammed_ElAfifi at yahoo.com> 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.