your spec file is autogenerated.
look at http://fedoraproject.org/wiki/PackagingGuidelines and edit your file by
hand.

(In reply to comment #1)
> your spec file is autogenerated.
> look at http://fedoraproject.org/wiki/PackagingGuidelines and edit your file by
> hand.

I modified gnome-yum.spec and create new src.rpm...

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-2.src.rpm

This is not a full review but a couple of comments:
- The consolehelper symlinks don't belong to %post/%postun scriptlets, package
them along with other content in %install
- You should add dependency on usermode(-gtk) for the above
- GConf2, vte, gnome-vfs2 dependencies are probably automatically detected by
rpmbuild, if so they should be removed as redundant
- Requires: rpm is silly - you must have rpm installed to install an rpm anyway
- Adding 3rd party repositories containing known legally problematic packages is
not allowed, the fedora-repositories subpackage and any references to those
repositories needs to be removed.
- The for-loop for copying the docs is redundant, just use
  %doc ABOUT-NLS AUTHORS README... in %files section
- I'd suggest using %find_lang macro instead of the current
{_datadir}/locale/*/LC_MESSAGES/gnome-yum.mo in %files

> - Requires: rpm is silly - you must have rpm installed to install an rpm anyway

not necessarily; chroot installations do not need rpm

- Okay. I remove all repositories from source package and I insert %find_lang
macro to the spec file.
- I can't solve the consolehelper symlinks with your solving and I don't modify
this problem in the end.
(error message:
   RPM build errors:
   Symlink points to BuildRoot: /usr/bin/gnome-yum =>
/var/tmp/gnome-yum-0.1.2-3-root-root/usr/bin/consolehelper)

Here is the next release:

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-3.src.rpm

The URL for your spec is pointing to an old version, could you update it please?

- Missing BR: gtkhtml2-devel

(In reply to comment #3)
> - Requires: rpm is silly - you must have rpm installed to install an rpm anyway

Strictly speaking you need to have rpm-libs and a front-end. What's interesting
is that I don't see any path from gnome-yum to rpm-libs. Am I missing something
obvious?

BuildArch:       i386
an reason for that? what about x86_64 ?

(In reply to comment #6)
> The URL for your spec is pointing to an old version, could you update it please?

Use 'Refresh' button... I used it too.

(In reply to comment #8)
> BuildArch:       i386
> an reason for that? what about x86_64 ?

Huh! Truly. I remove it. Here is the 4th release:

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-4.src.rpm

(In reply to comment #9)
> (In reply to comment #6)
> > The URL for your spec is pointing to an old version, could you update it please?
> 
> Use 'Refresh' button... I used it too.

I doubt that was the problem, since it was the first time I visited.  But I see
the right version now, so thanks.

(In reply to comment #5)
> - I can't solve the consolehelper symlinks with your solving and I don't modify
> this problem in the end.
> (error message:
>    RPM build errors:
>    Symlink points to BuildRoot: /usr/bin/gnome-yum =>
> /var/tmp/gnome-yum-0.1.2-3-root-root/usr/bin/consolehelper)

There are two ways of doing this currently implemented in Extras

yumex in Extras has the symlinks created inside the Makefile in the source.
netgo in Extras uses a construction in the specfile in the %install section:

take a look at both the src.rpm for both yumex and netgo and choose a method
that best suits you.  Don't forget to add Requires: usermode  to make sure
consolehelper is installed on the system as a dependancy.

-jef

(In reply to comment #11)

I resolved this problem in the next release:
Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-5.src.rpm

Missing build requires: gtkhtml2-devel (config fails without it)

(trying srpm from comment #12)
rpmlint isn't so happy either:
E: gnome-yum no-changelogname-tag
E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/README
W: gnome-yum no-dependency-on usermode-consoleonly
E: gnome-yum non-executable-script /usr/share/gnome-yum/gyum-query.sh 0644
E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/NEWS
W: gnome-yum non-conffile-in-etc /etc/pam.d/gnome-yum
E: gnome-yum zero-length /usr/share/doc/gnome-yum-0.1.2/TODO
W: gnome-yum non-conffile-in-etc /etc/security/console.apps/gnome-yum

Changelog is just completely missing .. shouldn't be :-)

Empty file warnings are easy to fix, just leave them out :-) The non-executable,
maybe a root,755,root %attr for it? 

The warning about the non conf file can be ignored, since it is a config file 

I see the spec file uses:
[ -n "$RPM_BUILD_ROOT" -a "$RPM_BUILD_ROOT" != "/" ] && rm -rf $RPM_BUILD_ROOT
This was a standard (a good while ago) but the current guidelines say to just
use: rm -rf $RPM_BUILD_ROOT

If no one else has stepped up for review yet, i'll be willing too, changing
blocker to FE-REVIEW. Fix the issues mentioned in the comments (including this
one :-)) and once thats done i'll do the full review

It would also be ok to do:
%{_datadir}/pixmaps/gnome-yum

in the files list, no need to type out all the file names really, and otherwise
you wouldn't own the directory

for the config file mentioned above, try putting %config(noreplace) before them
(pam.d/gnome-yum and console.apps/gnome-yum), which is proper thing to do with
config files

also it might be usefull to put gettext in the build requires, while i've never
seen a live fedora system without, mock does need to know about it else it might
trip over it

Desktop file should be parsed with desktop-file-install, adding the Fedora
category, and add desktop-file-utils to the buildrequires

Lastly there's no need for the: [ ! -f Makefile ] || make distclean
line is there? If gnome-yum does its releases properly it should never be
shipped with a Makefile

I repair (Comment #13-#15) bugs (exception rpmlint warning: 'W: gnome-yum
no-dependency-on usermode-consoleonly') and create a new package:

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-6.src.rpm

Few last comments before i can do a complete formal review checklist:

- Its not Buildrequires but: BuildRequires: (notice the capital R)

- %changelog doesn't have to include the upstream (source) changes, but the
changes you made to the package, ie:

* Sat Jan 14 2006 András Tóth  <toth_bandi.net> - 0.1.2-6 
- Fixed desktop file vendor
- Owned complete datadir
- Changed clean section to be fedora compliant
- find_lang can use %{name} macro, doesn't need to hard-code 'gnome-yum'

And so on .. Your expected to add such a changelog entry for every 'release' of
the package you make (ie up the version or release field) describing what you
changed in the specfile. Its not required to list what changed in 'gnome-yum'
its self,only what you changed of the package / specfile

- attr for gyum-query.sh doesn't have to go thru a defattr, you could just do:
%attr(0755,root,root) %{_datadir}/gnome-yum/gyum-query.sh 

- desktop file install misses:
   --add-category X-Fedora

- Missing BuildRequires entry: libgnomeui-devel

Thats it i think, great progress so far! Once these final issues are resolved
i'll give it the final formal run thru

Last 2 small comments: 

- No ; needed after the done (in the for doc ... loop), also no ; needed after
the  rm -f line

- Could you please add:
   %dir %{_datadir}/pixmaps/gnome-yum
before the: %{_datadir}/pixmaps/gnome-yum line?

Thats all i could find :-)

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.2-7.src.rpm

Thanks that solves all the issues reported

However it looks like you modified the source.tar.bz2 file? MD5SUM now missmatches

md5sum:
43e4f678a9243c92297f0929e7d87efe  (original from sf.net) gnome-yum-0.1.2.tar.bz2
bc105a3711409e8bdd66522caa59b316  (from src.rpm) gnome-yum-0.1.2.tar.bz2

I presume you've done this to include the X-Fedora category? Might be easier to
use the original source and in your spec file do:

desktop-file-install --vendor fedora --delete-original       \                 
                                             
  --dir $RPM_BUILD_ROOT%{_datadir}/applications             \    
  --add-category X-Fedora \                                                    
       
  $RPM_BUILD_ROOT%{_datadir}/applications/gnome-yum.desktop    

It is a strict policy for Fedora Extra's that source tarbal must match the
original one (checked with md5sum) so unfortunatly i can't accept this package yet.

Please either update the upstream tarbal, or use the original one and add the
--add-category X-Fedora to the desktop-file-install

Other then that its looking great, and builds cleanly in mock too. I will give
it the formal review once the source tarbal issue is resolved.

I need modify the next files in the source package:
./configure.in and ./configure => remove gtkhtml2-devel checking
src/callbacks.c => remove one line: #include <libgtkhtml/gtkhtml.h>
./README,./TODO => to be not empty
./gnome-yum.spec => fix format bugs
./desktop.in => Add fedora category

If it is necessary I make a new release (v0.1.3). It is OK?

Making a 1.3.0 release would be preferable then yes

Or alternativly you could make a patch file and include it in the spec/rpm that
makes the changes you mentioned, but since your able to change this upstream,
making a new release would be easier yes :-)

As soon as you have that ready and with a new .spec / .src.rpm i'll give it the
formal review.

Thanks for the effort so far!

Spec Name or Url: http://gnome-yum.sourceforge.net/download/gnome-yum.spec
SRPM Name or Url:
http://gnome-yum.sourceforge.net/download/gnome-yum-0.1.3-1.src.rpm

Looks good, the formal review:

MUST review items:
- Builds cleanly on FC5 devel.
- rpmlint has no output / complaints
- Source included matches upsteam source (md5sum)
- Package name meets guidelines
- spec file name is in %{name}.spec format
- Licence (GPL) is fedora extra's compatible & is included in spec
- Spec file is in (american) english
- Does not list buildrequires that are excepted in the package guidelines
- All build dependencies are listed
- No ldconfig needed
- All files have proper permissions
- Package is not relocatable
- No duplicate files in %files section
- No missing files in %files section
- Has a proper %clean section with rm -rf $RPM_BUILD_ROOT
- Uses macro's described in PackagingGuidelines
- No entries in %doc that are required for standard program operation
- No -devel package needed
- No directory-ownerships needed

Should items:
- Includes upstream licence file (COPYING)
- No insane scriplets
- No unnescesarry requires

rpmlint's only warning is:
W: gnome-yum no-dependency-on usermode-consoleonly

Mock builds cleanly for fc-devel

Please read:
- http://fedoraproject.org/wiki/Extras/NewPackageProcess and
- http://fedoraproject.org/wiki/Extras/Contributors and 
- http://fedoraproject.org/wiki/Extras/UsingCvsFaq

Carefully for the next steps


FE-APPROVED

Any particular reason why this package is not yet imported and built ?

Most likely because I just sponsored the packager this past weekend.

Next time please add a _valid_ email address in CVS owners.list,
where "valid" means it is used in your bugzilla account!

Why the toth_bandi.net isn't valid e-mail address? 

Okay. I tested this address and really it isn't working. The problem is the mail
system of sourceforge.net. I am waiting any times and if it is necessary I shall
change e-mail address...

> Why the toth_bandi.net isn't valid e-mail address? 

You added a different address to owners.list:

https://www.redhat.com/archives/fedora-extras-commits/2006-April/msg00137.html
https://www.redhat.com/archives/fedora-extras-commits/2006-April/msg00138.html

package is in the repos. Clsoing bug. 

Please remember to close package reviews once they have been imported into cvs
etc etc