Slightly changed spec-file.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

* version-release tags should be added to the changelog entry(s) (at least to 
  your ones)
* package checks for 'valgrind' (which is available). afais, it is used in an  
  assertion function only but perhaps it should be added to the BuildRequires?
* 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
  which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
  package?
* upstream ships a GPG signature which should be added to the package (as 
  additional Source) to ease verification of the tarball
* there should be a

  | Requires: /usr/lib/pkgconfig
  or
  | Requires: pkgconfig

  in the -devel package. First variant would be more correct, but can cause
  ambiguities with some core packages 

> * version-release tags should be added to the changelog entry(s) (at least to 
>   your ones)

Done.

> * package checks for 'valgrind' (which is available). afais, it is used in an 
>   assertion function only but perhaps it should be added to the BuildRequires?

Hmmm. It's optional. Would it be more preferable to allow the app to choose
itself (at the './configure'-stage) whether to use Valgrind?

> * upstream ships a GPG signature which should be added to the package (as 
>   additional Source) to ease verification of the tarball

Source1: http://arg0.net/users/vgough/download/rlog-1.3.7.tgz.asc

Done.

> * there should be a
>  | Requires: /usr/lib/pkgconfig
>  or
>  | Requires: pkgconfig

I *temporary* choose second variant. What ambiguities did you mention about? I
don't see any.

(In reply to comment #2)

> * 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
>   which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
>   package?

Done.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

> > * package checks for 'valgrind' (which is available). afais, it is
> >   used in an assertion function only but perhaps it should be
> >   added to the BuildRequires?
> 
> Hmmm. It's optional. Would it be more preferable to allow the app to
> choose itself (at the './configure'-stage) whether to use Valgrind?

To make builds reproducible, you should either add the BuildRequires:
or use '--disable-valgrind'. Because it does not seem to introduce
additional dependencies, I do not see reasons to disable it.


> > * there should be a
> >  | Requires: /usr/lib/pkgconfig
> >  or
> >  | Requires: pkgconfig
> 
> I *temporary* choose second variant. What ambiguities did you mention
> about? I don't see any.

'rpm -qf /usr/lib/pkgconfig' shows that this directories is owned by
several core packages. Because yum's depsolver is not very smart, it
will probably choose the wrong one which will perhaps add lots of
unwanted dependencies


> > * 'refman.pdf' comes with unresolved indices (toc chapter 4, Page 13 (resp. 9))
> >   which looks like a missing 'latex' run; perhaps doc should be rebuilt in the
> >   package?
> 
> Done.

ok; but
- you should add some more BuildRequires then (e.g. tetex-latex,
  doxygen).
- temporary pdf files (classrlog_*.pdf) should not be shipped but
  'refman.pdf' only


------------------

New issue

* 'INSTALL' should not be packaged; it does not contain anything which
  might be useful for the enduser

(In reply to comment #5)

> To make builds reproducible, you should either add the BuildRequires:
> or use '--disable-valgrind'. Because it does not seem to introduce
> additional dependencies, I do not see reasons to disable it.

OK, added valgrind to BuildRequires

> 'rpm -qf /usr/lib/pkgconfig' shows that this directories is owned by
> several core packages. Because yum's depsolver is not very smart, it
> will probably choose the wrong one which will perhaps add lots of
> unwanted dependencies

Done.
Requires: pkgconfig


> - you should add some more BuildRequires then (e.g. tetex-latex,
>   doxygen).

Done.

> - temporary pdf files (classrlog_*.pdf) should not be shipped but
>   'refman.pdf' only

Done.

> New issue
> 
> * 'INSTALL' should not be packaged; it does not contain anything which
>   might be useful for the enduser

Fixed.

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Ok, I APPROVE http://paula.comtv.ru/rlog-1.3.7-1.src.rpm for Fedora Extras

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (GNU/Linux)

iQEVAwUBQ3fJSAc3PX/XHFA7AQqG1gf+MygOhOt5++RnJaZPHQ9rPMBFKl74M9gE
4u2nNIP50rNpDzo7HEZ6P8uuMQQMoKh1m7MlkDORV/vBdOz9SPa5tlyYo/pbNRRO
62CxDi0yfmjlj1Deu+yn6g7SQNJ8791NxuT4iZdLCMha7l+OtUdV1NF03xmOK3PW
9MMG7VKElKVmY7Ie+3PQ1ZTyK0wT6VNPkgzFrlqq267qOlY9UFyXI5JfQh/yDax4
JTWZY48/T+JeZ7hNhCA0lBoXKtTfzqDQn9aQH3sWxmLWenL4urZDXI19nOmmekfR
a1nAWoDHQ5slyM0k0KXUNlNszT5dzgtLrM8Fmb+ZCA+TqG7GTIIjtA==
=HzNF
-----END PGP SIGNATURE-----

(In reply to comment #7)

> Ok, I APPROVE http://paula.comtv.ru/rlog-1.3.7-1.src.rpm for Fedora Extras

Thanks. Could you sponsor me?

(In reply to comment #6)
>> To make builds reproducible, you should either add the BuildRequires:
>> or use '--disable-valgrind'. Because it does not seem to introduce
>> additional dependencies, I do not see reasons to disable it.
> OK, added valgrind to BuildRequires

Is that valgrind only used for an optional build-time check?  One major drawback
of requiring valgrind during build is artificially limiting the architectures
that this can be included on.  valgrind's x86_64 support is very new, and it
doesn't run on ppc at all.  I would recommend disabling valgrind explicitly
during rpmbuild for this reason.

I would encourage you to please look into reviewing other contributor's packages
in order to demonstrate to the sponsors that you truly understand the packaging
guidelines and project processes, as that is the main thing we are looking for
in candidates for sponsorship.

I am considering sponsoring you soon, but I am looking more into your
participation first.

Also do you intend on submitting a fuse-encfs package?

Also don't forget to add the dist tag to the Release before this is imported.

valgrind should not be a problem; it is used for creating nice stacktraces only
without adding library deps. But I never looked whether these stacktraces are
working also on x86_64 and PPC.

How about ifarch it to include/exclude valgrind only if it is capable on that
architecture?

> I would recommend disabling valgrind explicitly
> during rpmbuild for this reason.

Added %ifarch check.

> Also do you intend on submitting a fuse-encfs package?
 
Yes. More to say - I trying to put rlog-package into extras especially because
of fuse-encfs :).

Spec Name or Url: http://paula.comtv.ru/rlog.spec
SRPM Name or Url: http://paula.comtv.ru/rlog-1.3.7-1.src.rpm

When updating your .src.rpm, please always increment the Release number and add
a changelog entry.

I will test your RPM...

OK APPRPROVED, and I am sponsoring you now.

Please CC me on the new bug when you have a fuse-encfs package for me to review.
 Thanks!

(In reply to comment #16)
> OK APPRPROVED, and I am sponsoring you now.

Thanks!
Added to CVS.

> Please CC me on the new bug when you have a
> fuse-encfs package for me to review.

OK.

Built on devel.

Heh, seems that it's my first review request! :)
It was almost 4 years ago. Anyway, back to serious things:


Package Change Request
======================
Package Name: rlog
New Branches: EL-4 EL-5
Owners: peter

congrats. ;) 

cvs done.

Package Change Request
======================
Package Name: rlog
New Branches: epel7
Owners: salimma
InitialCC:

*** Bug 1098460 has been marked as a duplicate of this bug. ***

Git done (by process-git-requests).