Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=226223
Summary: Merge Review: ORBit2 Product: Fedora Extras Version: devel Platform: All OS/Version: Linux Status: NEW Severity: normal Priority: normal Component: Package Review AssignedTo: nobody@fedoraproject.org ReportedBy: nobody@fedoraproject.org QAContact: fedora-package-review@redhat.com CC: mclasen@redhat.com
Fedora Merge Review: ORBit2
http://cvs.fedora.redhat.com/viewcvs/devel/ORBit2/ Initial Owner: mclasen@redhat.com
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ORBit2
https://bugzilla.redhat.com/show_bug.cgi?id=226223
bugzilla@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |medium Priority|normal |medium Product|Fedora Extras |Fedora
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |254211 nThis| | AssignedTo|nobody@fedoraproject.org |mtasaka@ioa.s.u-tokyo.ac.jp Status|NEW |ASSIGNED Flag| |fedora-review?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ORBit2
https://bugzilla.redhat.com/show_bug.cgi?id=226223
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-09-10 04:02 EST ------- For 2.14.7-6:
* License - Please recheck the license * Most source codes do not specify any version and it seems that source codes which contain license term contain "and any later" term. So at least this can be "LGPLv2+ and GPLv2+"
* Redundant requires - For devel -------------------------------------------------- Requires: glib2-devel Requires: glib2-devel >= %{glib2_version} -------------------------------------------------- The first line can be omitted.
* Parallel make - Support parallel make when possible.
* %makeinstall - Please avoid %makeinstall when possible. - Usually I recommend -------------------------------------------------- make install DESTDIR=$RPM_BUILD_ROOT INSTALL="%{__install} -p" -------------------------------------------------- for recent Makefile. This will keep timestamps on most files
* Directory ownership issue - Please fix up directory ownership issue. -------------------------------------------------- %_libdir/orbit-2.0 - must owned by main, not by devel %_datadir/idl - must owned by ORBit2-devel, not by libbonobo-devel
* Defattr - Now we recommend %defattr(-,root,root,-)
* Rpmlint - file-not-utf8 /usr/share/doc/ORBit2-2.14.7/NEWS * Please change the encoding to UTF-8
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ORBit2
https://bugzilla.redhat.com/show_bug.cgi?id=226223
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- OtherBugsDependingO| |225989 nThis| |
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ORBit2
https://bugzilla.redhat.com/show_bug.cgi?id=226223
------- Additional Comments From mtasaka@ioa.s.u-tokyo.ac.jp 2007-09-13 03:48 EST ------- Matthias, ping?
Please do not reply directly to this email. All additional comments should be made in the comments box of this bug report.
Summary: Merge Review: ORBit2
https://bugzilla.redhat.com/show_bug.cgi?id=226223
mtasaka@ioa.s.u-tokyo.ac.jp changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |NEEDINFO Flag| |needinfo?(mclasen@redhat.com | |)
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
Matthias Clasen mclasen@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flag|needinfo?(mclasen@redhat.co | |m) |
--- Comment #3 from Matthias Clasen mclasen@redhat.com 2008-12-10 23:52:43 EDT --- Should all be taken care of in the current build, I think
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 #4 from Mamoru Tasaka mtasaka@ioa.s.u-tokyo.ac.jp 2008-12-12 09:54:12 EDT --- For 2.14.16-2:
* Licensing
https://fedoraproject.org/wiki/Packaging/LicensingGuidelines#Multiple_Licens... - Please write in the spec file what files are actually licensed under GPLv2+. As far as I checked the source codes, the codes under GPLv2+ are: ------------------------------------------------------ src/idl-compiler/orbit-idl-driver.c src/idl-compiler/orbit-idl-main.c src/services/name/name-client.c test/*.c test/everything/*.c test/poa/*.c ------------------------------------------------------ and these affects files under %_bindir in ORBit2-devel.
So the license of ORBit2 (main package) should be only "LGPLv2+" and only -devel package should have "LGPLv2+ and GPLv2+" license tag.
! Timestamps make install DESTDIR=$RPM_BUILD_ROOT INSTALL="install -p" is preferrred to keep timestamps on installed files.
* Requires - Would you explain why -devel subpackage should have "Requires: indent"?
* Documents - Please add the following file(s) into main package ------------------------------------------------------ COPYING.LIB ------------------------------------------------------ - Maybe the following file(s) can be added in -devel subpackage ------------------------------------------------------ ChangeLog MAINTAINERS HACKING ------------------------------------------------------
* Shipping static archive
https://fedoraproject.org/wiki/Packaging/Guidelines#Packaging_Static_Librari... - Split static archives into -static subpackge so that we can keep track of packages really needing the archive.
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(पराग) panemade@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |panemade@gmail.com
--- Comment #5 from Parag AN(पराग) panemade@gmail.com 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.
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 mtasaka@ioa.s.u-tokyo.ac.jp 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_...
* 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_Librari... - 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 -------------------------------------------------------------------------------
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(पराग) panemade@gmail.com 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
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(पराग) panemade@gmail.com 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.
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 limburgher@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |limburgher@gmail.com AssignedTo|mtasaka@fedoraproject.org |limburgher@gmail.com
--- Comment #9 from Jon Ciesla limburgher@gmail.com 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.
package-review@lists.fedoraproject.org
-
Red Hat Bugzilla