* Probably better summary: "Yet Another FTP/SFTP Client"
* Better to take the description from http://sourceforge.net/projects/yafc:
  "Yafc is an OpenSource console mode FTP client. It has support for Kerberos 4/5  
   authentication and sftp (ssh2). Other features include tab completion,
   directory cache, powerful aliases, recursive file commands and bookmarks with
   autologin."
* fails to build in mock:
  RPM build errors:
    File not found: /var/tmp/yafc-1.1.1-1-root-mockbuild/usr/share/info/dir

  The dir file has not been created. Instead of "%exclude", simply
  use "rm -f" in the %install section, this works even if dir has not
  been created.
* Normally "install-info --delete" goes into %postun instead of %preun
* You don't need "Req: readline", rpmbuild takes care of this
* Simply removing the static keyworkd in gssapi.c makes
  it compile with kerberos enabled
* I don't think, that it is a good idea to enable color always. This
  does only work in ANSI terminals, it is a mess in dumb terminal, for
  example with emacs. In fact the doc states that, just as the real ls,
  you have to add the --color option to ls to enable color.

  

I've fixed most of the concerns (haven't posted yet since I want to address the
other concerns, first).

I don't know what you mean about the fix for kerberos.  I think you meant to
type "static keywords" but don't know which ones you're talking about.  I don't
know much about compiling things, so I'm having a hard time figuring out what
you mean.  I'm also asking upstream, in hope that they can just fix the problem
so I don't need to maintain a patch.

Created attachment 132127 [details]
Patch for compiling with krb5

Here is the patch for compiling with krb5.
You need of course add krb5-devel to the BR.

spec:  http://rpm.forevermore.net/yafc/yafc.spec
srpm:  http://rpm.forevermore.net/yafc/yafc-1.1.1-3.src.rpm

ok, patch, etc applied.  Builds in mock, too.

rpmlint result:
E: yafc zero-length /usr/share/doc/yafc-1.1.1/doc/yafc.info

The package does indeed contain an empty compressed file yafc.info.gz.

(In reply to comment #5)
> rpmlint result:
> E: yafc zero-length /usr/share/doc/yafc-1.1.1/doc/yafc.info
> 
> The package does indeed contain an empty compressed file yafc.info.gz.

The cause is building triggers regeneration of the *.info while "makeinfo" is
missing.

/builddir/build/BUILD/yafc-1.1.1/support/missing: line 46: makeinfo: command not
found
WARNING: `makeinfo' is missing on your system.  You should only need it if
         you modified a `.texi' or `.texinfo' file, or any other file
         indirectly affecting the aspect of the manual.  The spurious
         call might also be the consequence of using a buggy `make' (AIX,
         DU, IRIX).  You might want to install the `Texinfo' package or
         the `GNU make' package.  Grab either from any GNU archive site.

=> BR: /usr/sbin/makeinfo
could be applied to work around this issue.


But .. the actual cause is deeper: The tarball is not packaged properly.
It contains a raw preconfigured CVS snapshot with all temporary files and broken
timestamps inside.

Due to this I strongly recommend to add a 
make distclean
to %prep to assure the subsequent %configure doesn't "reconfigure" the source
tree, but to configure it anew.

Further issues:

* You put *.texi's in %doc. These *.texi's are the sources of yafc.info. It
doesn't make much sense to put them into %docdir

* Related to the previous issue, the "prepare for %doc" block doesn't seem
useful to me. The package ships and installs mans and infos, its Makefiles
handle them correctly. There is no for any special treatment.

I think I addressed the requested issues.  rpmlint is quiet now, too.

spec:  http://rpm.forevermore.net/yafc/yafc.spec
srpm:  http://rpm.forevermore.net/yafc/yafc-1.1.1-4.src.rpm

Gérard or Ralf: Are either of you going to do a formal review on this package?

If not, I would be happy to step in and do so...

(In reply to comment #8)
> Gérard or Ralf: Are either of you going to do a formal review on this package?
> 
> If not, I would be happy to step in and do so...
Yes, go on.

OK - Package name
OK - Spec file matches base package name.
OK - Meets Packaging Guidelines.
OK - License (GPL)
OK - License field in spec matches
OK - License file included in package
OK - Spec in American English
OK - Spec is legible.
OK - Sources match upstream md5sum:
832d074183a36ee15b47553ed5962fce  yafc-1.1.1.tar.bz2
832d074183a36ee15b47553ed5962fce  yafc-1.1.1.tar.bz2.1
OK - Package compiles and builds on at least one arch.
OK - BuildRequires correct
OK - Package owns all the directories it creates.
OK - Package has no duplicate files in %files.
See below - Package has %defattr and permissions on files is good.
OK - Package has a correct %clean section.
OK - Spec has consistant macro usage.
OK - Package is code or permissible content.
OK - Packages %doc files don't affect runtime.
OK - Package doesn't own any directories other packages own.
OK - No rpmlint output.

SHOULD Items:

OK - Should include License or ask upstream to include it.
OK - Should build in mock.

Issues:

1. You might change your default attributes from:
%defattr(-, root, root)
to
%defattr(-, root, root,-)

Aside from that I see no issues.
Feel free to change issue #1 above when you check the package in.

Pending that, this package is APPROVED.

Which license?  The GPL is there already (document is called COPYING, which it
is in many GPL programs).  Package builds fine in mock in my system, too.

Anyway, issue #1 is fixed, and I will add the package to extras soon.

In reply to comment #11: 

Yeah, thats my checklist of items when looking at packages... 
the OK there means that they License was included and is ok. :) 

Might as well get all of my good packages into EPEL:

Package Change Request
======================
Package Name: yafc
New Branches: EL-5

cvs done.