Note: This is a public test instance of Red Hat Bugzilla. The data contained within is a snapshot of the live data so any changes you make will not be reflected in the production Bugzilla. Email is disabled so feel free to test any aspect of the site that you want. File any problems you find or give feedback at bugzilla.redhat.com.
Bug 173040
Summary: | Review Request: rlog - Runtime Logging for C++ | ||
---|---|---|---|
Product: | [Fedora] Fedora | Reporter: | Peter Lemenkov <lemenkov> |
Component: | Package Review | Assignee: | Enrico Scholz <rh-bugzilla> |
Status: | CLOSED NEXTRELEASE | QA Contact: | David Lawrence <dkl> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | rawhide | CC: | fedora-extras-list, michel, rh-bugzilla, wtogami |
Target Milestone: | --- | Flags: | gwync:
fedora-cvs+
|
Target Release: | --- | ||
Hardware: | All | ||
OS: | Linux | ||
URL: | http://arg0.net/wiki/rlog | ||
Whiteboard: | |||
Fixed In Version: | Doc Type: | Bug Fix | |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2005-12-16 19:31:13 UTC | Type: | --- |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: | |||
Bug Depends On: | |||
Bug Blocks: | 163779, 1098472 |
Description
Peter Lemenkov
2005-11-12 21:55:28 UTC
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). |