Bug 713122

Blocking FE-NEEDSPONSOR.

Please don't put trademarks into the Summary like that (even if you misspell them ;-) ). Try to describe what the package objectively DOES instead.

okay, will be changed with next release. Thanks for advice.

Update:
Spec URL: https://dl.dropbox.com/s/zjkybrv5vywsgod/flyback.spec?dl=1
SRPM URL: https://dl.dropbox.com/s/sabesc4eb8aafay/flyback-0.6.5-2.fc15.src.rpm?dl=1

Update:
Spec URL: https://dl.dropbox.com/s/zjkybrv5vywsgod/flyback.spec?dl=1
SRPM URL: https://dl.dropbox.com/s/06n9mj1w2zwawe8/flyback-0.6.5-3.fc15.src.rpm?dl=1

Just some quick comments:

- You are mixing macro style and variable style (${RPM_BUILD_ROOT} vs.  %{buildroot})
- Comment the patches
  https://fedoraproject.org/wiki/Packaging:Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
-

Here are some further comments:

- If you use sources from an scm, always checkout a specific revision or 
  changeset, e.g.
  hg clone -u 80e92ef119a0 https://flyback.googlecode.com/hg/ flyback

  Otherwise, it's hard to verify the integrity of the sources. It would be 
  even better if upstream released source tarballs and not just deb files. 
  Maybe you can ask him to do this.

- As Fabian already wrote, add comments above each PatchX line telling what 
  the patch does.
  Also, to reduce the number of patches, merge all patch files that have 
  similar goals, e.g. fixing paths.

- There's no copyright information in the source files, so the license is
  GPL+. You should ask upstream to add a copyright header to the source files
  as mentioned in the GPL license text. The version information in the
  enclosed GPL.txt does not automatically apply to the sources.

- Please use plain shell commands instead of the corresponding macros:
  http://fedoraproject.org/wiki/PackagingGuidelines#Macros

- As there's no BuildRoot field in your spec file, you probably don't plan to 
  support EPEL < 6. Thus, you can also drop the %clean section.

- replace %defattr(-,root,root) with %defattr(-,root,root,-)

- Please be more specific in %files, i.e. prefer complete or partial 
  filenames over a generic "*" (escpecially, if only a single file is added).

- Drop the asterisk from %{_datadir}/%{name}/* for proper directory ownership.

- Why do you need the shell script "flyback"? Wouldn't be a symlink 
  sufficient?

I'm very busy atm but as soon as I've got more free time I'll try to apply your suggestions. But in the Meantime you could tell me what's the best way to create a symlink in a specfile ;-)

(In reply to comment #8)
> But in the Meantime you could tell me what's the best way to
> create a symlink in a specfile ;-)

Sure. Something like this in %install should do the job:
ln -s %{_datadir}/%{name}/flyback.py %{buildroot}%{_bindir}/flyback

Next attemp :-)
SRPM: https://dl.dropbox.com/s/azkdtzi9o2v1lc9/flyback-0.6.5-4.fc15.src.rpm?dl=1
SPEC: https://dl.dropbox.com/s/zjkybrv5vywsgod/flyback.spec?dl=1

You're still using macros for mkdir and install. Replace that with commands. As Martin also mentioned, the clean section is still there.

The defattr line was found to be no longer necessary some time ago.

"Apple's Time Machine for Linux" -- That comment in the desktop file is inacceptable. See http://fedoraproject.org/wiki/PackagingGuidelines#Trademarks_in_Summary_or_Description

The chmod doesn't seem to do anything.

Next attemp :-)
SRPM: https://dl.dropbox.com/s/4f44e4lgexyyt53/flyback-0.6.5-5.fc15.src.rpm?dl=1
SPEC: https://dl.dropbox.com/s/zjkybrv5vywsgod/flyback.spec?dl=1

You're requiring Python twice.

You still have the clean section, you're still having the install macros. Defattr is still not necessary.

Changelog entries like "minor changes" and comments like "fix some paths" are pointless, in my opinion. Please be a bit more concrete!

Heiko, please remove 'StalledSubmitter' from the whiteboard when you performed the changes suggested in comment:13.

SRPM:
https://dl.dropbox.com/s/rxj42ovrkp43ydg/flyback-0.6.5-5.fc16.src.rpm?dl=1
SPEC: https://dl.dropbox.com/s/zjkybrv5vywsgod/flyback.spec?dl=1

Some preliminary comments:

- There is a spelling error: it's 'deprecated' and not 'deprecheated'
- Leave the build section empty and add a comment that it is not needed instead of commenting it out. This will remove the following rpmlint warning:flyback.spec: W: no-%build-section
- This seems to be wrong/not needed. Why do you do this?
 #add python headlines to python-scripts
for file in src/*.py; do
   sed -i -e '1i#!/usr/bin/env python2' $file
done
- Why do you still use %{__install}?
 %{__install} -p -m 755 src/*.py %{buildroot}%{_datadir}/%{name}/

- The mode in the install command is wrong for all files except flyback.py. The other files do not need to be executable, therfore mode 644 is correct.

- Actually you do not need to require python at all, because of the shebang in flyback.py /usr/bin/python will already be required automatically

- The license is still problematic. According to the Google Project Page it is GPLv2 and not GPL+. Since the actual source does not contain any license header, it is afaik not even properly licensed as GPL+. Would you please ask upstream to add license headers as documented in http://www.gnu.org/licenses/gpl-howto.html ?

- It seems that flyback upstream is not active anymore. Is this package really worht being added to Fedora?

And please perform informal reviews and link to them here as proposed in http://fedoraproject.org/wiki/How_to_get_sponsored_into_the_packager_group#Reviewing_packages

You're right. Upstream seems to be allmost dead. So for me there isn't any problem to move this review request to /dev/null