[Bug 226223] Merge Review: ORBit2
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226223
Jon Ciesla changed:
What|Removed |Added
CC||limburg...@gmail.com
AssignedTo|mtas...@fedoraproject.org |limburg...@gmail.com
--- Comment #9 from Jon Ciesla 2012-04-09 14:41:04 EDT
---
Fresh review:
- rpmlint checks return:
ORBit2.spec:389: W: macro-in-%changelog %{_datadir}
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build. Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted. Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.
ORBit2.spec:551: W: macro-in-%changelog %files
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build. Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted. Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.
ORBit2.spec:551: W: macro-in-%changelog %{prefix}
Macros are expanded in %changelog too, which can in unfortunate cases lead to
the package not building at all, or other subtle unexpected conditions that
affect the build. Even when that doesn't happen, the expansion results in
possibly "rewriting history" on subsequent package revisions and generally odd
entries eg. in source rpms, which is rarely wanted. Avoid use of macros in
%changelog altogether, or use two '%'s to escape them, like '%%foo'.
Trivial to fix.
Some no man page and wrong fsf address errors, fix if feasible.
- package meets naming guidelines
- package meets packaging guidelines
- license ( LGPLv2+ and GPLv2+ ) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86_64)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
- devel requires base package n-v-r
So trivial things, and Parag's comments on patches.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 226223] Merge Review: ORBit2
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226223 --- Comment #8 from Parag AN(पराग) 2011-01-07 01:02:27 EST --- Unfortunately, I have no time to follow this package. Only thing I guess remaining here is to make sure patches have upstream bugzilla reported and here is only Patch1 need to have bug id and also %check thing. Removing from this package review work. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 226223] Merge Review: ORBit2
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226223 --- Comment #7 from Parag AN(पराग) 2010-09-17 05:50:34 EDT --- Just to update here, I am working on this review, meanwhile some modification are available at SRPM: http://paragn.fedorapeople.org/ORBit2-2.14.18-3.fc15.src.rpm SPEC: http://paragn.fedorapeople.org/ORBit2.spec -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 226223] Merge Review: ORBit2
Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.
https://bugzilla.redhat.com/show_bug.cgi?id=226223
--- Comment #6 from Mamoru Tasaka 2010-09-09
13:15:19 EDT ---
Well, checking master head (2.14.18-2.fc15)
* License
- So, if you want to use "LGPLv2+ and GPLv2+" for license tag, please
specify explicitly which part is LGPLv2+ and which part is GPLv2+.
* Version specific BR
- ">=" parts on BuildRequires's are no longer needed because packages
on currently supported Fedora branches all satisfy these dependencies
( And especially I see no reason to keep writing these version specific BR
on current rawhide )
https://fedoraproject.org/wiki/Packaging/Guidelines#Explicit_Requires
* Status of the patches
- Please write the status of the patches (i.e. if the patch is to be merged
into the upstream, or if it is Fedora specific, or something else).
! Especially, I guess maintainers of ORBit2 on Fedora also take part in
GNOME Project, so I cannot guess why 2.14.3- patches are not yet merged
into 2.14.18 tarball.
https://fedoraproject.org/wiki/Packaging/Guidelines#All_patches_should_have_an_upstream_bug_link_or_comment
* Requires on -devel subpackage
- Would you explain what "R: indent" is for?
- "R: pkgconfig" "R: foo-devel" are no longer needed. These dependencies
are automatically needed.
! And as I wrote above, I think keeping writing explicit version
dependencies here
makes no sense.
* Duplicate %description
- %description in -devel subpackage contains the same contents shown in the
%description
of the main package and it is redundant.
* Timestamps
- Please consider to add "INSTALL='install -p'" option to "make install" to
keep
timestamps on installed files as much as possible, especially on installed
header
files.
* Static archive
- Please take care of
https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Libraries
- At least .a files must be split out from -devel subpackage
* %check
- As this package contains test/ directory, please add %check section and
execute
some test program (like $ make check ) there
! And (on my local machine) actually currently "make check" fails like
---
mem usage prev/post: 1388KB / 1464KB -- 1000x test_ORBit_alloc ()
**
ERROR:test-mem.c:239:main_func: assertion failed: (mem_usage_end -
mem_usage_start < 50)
/bin/sh: line 5: 30139 Aborted ${dir}$tst
FAIL: test-mem
1 of 6 tests failed
Please report to http://bugzilla.gnome.org/enter_bug.cgi?product=ORBit2
---
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are the QA contact for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 226223] Merge Review: ORBit2
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug. https://bugzilla.redhat.com/show_bug.cgi?id=226223 Parag AN(पराग) changed: What|Removed |Added CC||panem...@gmail.com --- Comment #5 from Parag AN(पराग) 2010-09-08 12:02:51 EDT --- mtasaka, Can you please review current rawhide package again? I will do the necessary changes in git. I too see -static should be generated. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the QA contact for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
