Odd; this failed to build for me in my local mock, and it fails in the buildsys in the same way:
  http://koji.fedoraproject.org/koji/taskinfo?taskID=886764
I wonder what could have changed in the last four days.

Oh, somehow your scratch build is of version 1.6, but the package you've posted is version 1.4.

I upload to my web old version by mistake.

Correct files:
SRPM: http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon-1.6-1.f10.src.rpm
Spec URL: http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon.spec

The specfile seems to still be 1.4, but I can pull it from the srpm.

Is 1.6 an actual release?  Because that doesn't seem to make sense when the only way to get it is to check it out of git.

You need to provide instructions for actually checking the packaged version out of git, not just what's currently at the head of the tree.

I know you probably depend on the names now, but is there really any point in postfixing the executable names with ".pl"?  It's up to you, of course, and this wouldn't be the first package to do this, but I've never understood why anyone would care what language a particular executable is written in.

I note this package provides perl(Satcon) = 1.3, which is... odd.  The module version differs from the package version?

>Is 1.6 an actual release?  Because that doesn't seem to make sense when the
>only way to get it is to check it out of git.

Yeah, perl-Satcon-1.6-1 is last release. See http://miroslav.suchy.cz/spacewalk/gitstat/tag.php for list of recent tags.

>You need to provide instructions for actually checking the packaged version out
>of git, not just what's currently at the head of the tree.

I did. See this comment inside spec:
# This src.rpm is cannonical upstream
# You can obtain it using this set of commands
# git clone git://git.fedorahosted.org/git/spacewalk.git/
# cd projects/perl-Satcon
# make srpm
The provided Makefile will checkout the git to the tag perl-Satcon-1.6-1 (commits after this tag are ignored) and make srpm from that version.

> I know you probably depend on the names now, but is there really any point in
> postfixing the executable names with ".pl"?
You know... I did not write that code. I just take the code as is and try to include it in Fedora. The code is several years old. And yeah, several thing we are doing different today. But it works, and I'm trying to as little as possible, during review. 

> I note this package provides perl(Satcon) = 1.3, which is... odd.  The module
> version differs from the package version?
Yeah there was hardcoded version in module. I removed it:

Updated SPEC:
http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon.spec
Updated SRPM:
http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon-1.7-1.f10.src.rpm

(In reply to comment #4)

> The provided Makefile will checkout the git to the tag perl-Satcon-1.6-1
> (commits after this tag are ignored) and make srpm from that version.

No, it will checkout whatever git tag the makefile in git head happens to specify.  If six months from now, it is altered to check out version 2.0 then those instructions will no longer be correct.  In fact, didn't you just alter it to check out 1.7?  How would I now check out 1.6?

> You know... I did not write that code. I just take the code as is and try to
> include it in Fedora. The code is several years old.

Well, it is my job as the reviewer to ask those questions.  It's OK if the names need to be that way because they've been that way for ages and other software expects them.

> Yeah there was hardcoded version in module. I removed it:

It is normal for Perl modules to indicate their versions, but I don't see any problem if you don't want to do that, although you then cannot have version-specific "use" statements in code which uses this module (and hence no automatic version-specific rpm dependency generation).  I doubt that would make any difference to your application, though.

I will finish up this review when I get home.

Well, that took longer than I intended.  Here are my comments:

It takes about 30 minutes to check out the git tree.  Is there any way to check out a subtree?

The checkout instructions are slightly wrong; you need to cd into the "spacewalk" directory as well, and the instructions generate an srpm, not a tgz.  We need instructions for generating the tgz.  A "git archive" instruction would probably work.  Honestly this would all be much simpler if someone just made a tarball and stuck it on an appropriate web site.

I confirmed my comments above; "make srpm" will indeed make an srpm of whatever version of perl-Satcon happens to be in git head.

The packager is supposed to bug the upstream developers if there's no copy of the license text included in the package.  In this case, you're the upstream so I'll just ask you to please consider including the text of the license you use in your tarball.

Really my only concern is the issue of duplicating the source tarball

* source files match upstream (compared manually from git checkout).
* package meets naming and versioning guidelines.
* specfile is properly named, is cleanly written and uses macros consistently.
* summary is OK.
* description is OK.
* dist tag is present.
* build root is OK.
* license field matches the actual license.
* license is open source-compatible.
* license text not included upstream.
* latest version is being packaged.
* BuildRequires are proper.
* %clean is present.
* package builds in mock (rawhide, x86_64).
* package installs properly.
* rpmlint is silent.
* final provides and requires are sane:
   perl(Satcon)
   perl-Satcon = 1.7-1.fc10
  =
   /usr/bin/perl
   perl(:MODULE_COMPAT_5.10.0)
   perl(Data::Dumper)
   perl(File::Copy)
   perl(File::Find)
   perl(File::Path)
   perl(File::Temp)
   perl(Getopt::Long)
   perl(RPM::Specfile)
   perl(Satcon)
   perl(bytes)
   perl(lib)
   perl(strict)

* %check is present and the minimal test suite passes.
* owns the directories it creates.
* doesn't own any directories it shouldn't.
* no duplicates in %files.
* file permissions are appropriate.
* no generically named files
* code, not content.
* documentation is small, so no -doc subpackage is necessary.
* %docs are not necessary for the proper functioning of the package.

I created tgz and uploaded it to fedorahosted.org.
I added LICENSE file with copy of GPLv2 as well.

Updated SPEC:
http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon.spec
Updated SRPM:
http://miroslav.suchy.cz/fedora/perl-Satcon/perl-Satcon-1.8-1.f10.src.rpm

Sorry for the slow response; I've been away for a few days.

Yes, this looks fine.

APPROVED

New Package CVS Request
=======================
Package Name: perl-Satcon
Short Description: Framework for configuration files
Owners: msuchy
Branches: devel F-9 F-10 EL-4 EL-5 
InitialCC:
Cvsextras Commits: yes

cvs done.

Any reason this hasn't been built yet?

Packages are built.