Bug 225957 - Merge Review: k3b
Merge Review: k3b
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Ed Hill
Fedora Package Reviews List
:
Depends On:
Blocks: 229420 Merge
  Show dependency treegraph
 
Reported: 2007-01-31 14:14 EST by Nobody's working on this, feel free to take it
Modified: 2007-11-30 17:11 EST (History)
3 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-06-20 10:31:42 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
ed: fedora‑review+
tcallawa: fedora‑cvs+


Attachments (Terms of Use)

  None (edit)
Description Nobody's working on this, feel free to take it 2007-01-31 14:14:20 EST
Fedora Merge Review: k3b

http://cvs.fedora.redhat.com/viewcvs/devel/k3b/
Initial Owner: harald@redhat.com
Comment 1 Ed Hill 2007-02-03 12:01:15 EST
Hi there, The following are the review items that stand out.  I realize 
that almost all of the includes are prefixed with "k3b" but nonetheless
it would be nice to put all of them in a subdir such as: /usr/include/k3b/

Anyway, the list of items is:

 - please use the preferred BuildRoot

 - rpmlint complains about a number of things:
   - devel content in non-devel package (many files)
     - please consider creating a sub-dir such as /usr/include/k3b
       to contain all the k3b headers
   - dead patches are still being carried around and should 
     probably be deleted:
W: k3b patch-not-applied Patch1: k3b-0.11.3-kde32.patch
W: k3b patch-not-applied Patch2: k3b-0.11.6-desktopfile.patch
W: k3b patch-not-applied Patch5: k3b-0.11.14-version.patch
W: k3b patch-not-applied Patch7: k3b-0.11.17-dao.patch
W: k3b patch-not-applied Patch6: k3b-0.11.14-suid.patch
W: k3b patch-not-applied Patch9: k3b-0.11.23-proxy.patch
W: k3b patch-not-applied Patch8: k3b-0.12.2-statfs.patch
   - please remove prereq-use /sbin/ldconfig
   - I don't really understand this--can someone else please 
     help explain it:
/tmp/k3b-0.12.17-1.i386.rpm.30099/usr/share/applications/kde-k3b.desktop:
warning: file contains key "DocPath", this key is currently reserved for use
within KDE, and should in the future KDE releases be prefixed by "X-"

 - please consider adding %{?dist} to Release
Comment 2 Ed Hill 2007-02-03 12:12:33 EST
Bummer.  I hit "save changes" a little too fast.  :-)  There are a few 
other things that should have been listed:

 + Please see builds, logs, and rpmlint output at:
http://linux.dell.com/files/fedora/FixBuildRequires/mock-results-core/i386/k3b-0.12.17-1.src.rpm/result/

 - the /usr/lib/kde3/*.la files should be deleted

 - some of the libs (e.g. /usr/lib/*.so) also belong in a separate
   -devel sub-package
Comment 3 Chitlesh GOORAH 2007-02-08 03:40:04 EST
(In reply to comment #2)
>  - the /usr/lib/kde3/*.la files should be deleted
 
Please ensure, that K3B really doesn't need it, else you will have a vague of
bug report from users. Since KDE3 itself is heavily dependent on .la files.
http://bugs.kde.org/show_bug.cgi?id=139445

If you are removing those .la files, ensure proper testing has been carried out.
Comment 4 Rex Dieter 2007-02-08 09:44:55 EST
>  the /usr/lib/kde3/*.la files should be deleted
I can vouge that in many cases, kde apps (still) do need these .la files.  
These are of the type that cause no harm (vs. those in %_libdir), so I'd 
strongly recommend leaving them as-is.

Re:  DocPath in .desktop file
Mostly harmless warning.  

While on the topic of .desktop files, I'd recommend using this instead (to 
preserve upstream .desktop vendor):
desktop-file-install --vendor="" \
  --dir $RPM_BUILD_ROOT%{_datadir}/applications/kde \
  --add-category X-Red-Hat-Base \
  $RPM_BUILD_ROOT%{_datadir}/applications/kde/%{name}.desktop
(not sure the point of adding X-Red-Hat-Base, but a comment justifying it's 
use in the specfile would be helpful for posterity).
Comment 5 Rex Dieter 2007-02-21 10:51:29 EST
Once merge is complete, *this* package can/should:
Obsoletes: k3b-extras < %{version}-%{release}
Provides:  k3b-extras = %{version}-%{release}
BuildRequires:  libmpcdec-devel
BuildRequires:  libsndfile-devel
and add %configure options:  --with-musepack --with-sndfile

See also bug #229420
Comment 6 Rex Dieter 2007-02-28 13:33:18 EST
harold, is this your package?  Can we have some movement here?  
We really need to get the kde stack merged by early next week.
Comment 7 Harald Hoyer 2007-03-02 08:04:47 EST
err... 
$ brew latest-pkg dist-fc7 k3b
Build                                     Tag                   Built by
----------------------------------------  --------------------  ----------------
k3b-1.0.0-0.rc6.1.fc7                     dist-fc7              harald
Comment 8 Rex Dieter 2007-03-02 08:35:53 EST
So, I take it then that you incorporated the Ed's suggestions already?
Comment 9 Rex Dieter 2007-03-02 08:37:23 EST
Hrm, looking closer, turns out besides .la files, Ed didn't have any
suggestions.  Ed, anything else blocking approval here?
Comment 10 Ed Hill 2007-03-04 22:42:53 EST
Hi folks, my apologies for the slow response--have been quite busy with
${WORK}.

My two main suggestions were the removal of *.la files [which I now 
understand are needed by many KDE apps--sorry!], and the creation of 
a -devel subpackage.  I see that the -devel package was created on 
Feb 15th so thats good.

And I don't see anything else at odds with the packaging guidelines so 
its APPROVED.
Comment 11 Rex Dieter 2007-04-10 14:55:16 EDT
Harald,
per comment #5, could you add the Obsoletes k3b-extras at least for now (even
before the BR's and Provides) so that we can have a clean upgrade path and
remove k3b-extras from the repos?

After the merge, you can do the rest.  Sound ok?
Comment 12 Harald Hoyer 2007-04-11 03:58:56 EDT
> After the merge, you can do the rest.  Sound ok?

No :) Need your experience and help :)
Comment 13 Rex Dieter 2007-04-11 07:38:05 EDT
OK, let me rephrase... 
After the merge, if you don't mind, I can help co-maintain k3b, and then *we*
can do the rest.
Sound better? :)

Regardless, we'd like to get the Obsoletes in place asap (ie, in time for f7t4).
Comment 14 Harald Hoyer 2007-04-11 08:53:36 EDT
k3b-1.0-1.fc7
Comment 15 Rex Dieter 2007-05-29 11:52:41 EDT
Hrm, we missed actually adding the BR's after the merge. :(

Harold, if you can add me as comaintainer (and add me to pkg.acl), I can help
out here.
Comment 16 Harald Hoyer 2007-05-30 08:18:21 EDT
Package Change Request
======================
Package Name: k3b
Updated Fedora Owners: harald@redhat.com,rdieter@math.unl.edu
Comment 17 Tom "spot" Callaway 2007-05-31 10:18:27 EDT
cvs done.
Comment 18 Rex Dieter 2007-06-20 10:31:42 EDT
Closing, all is well in the world.

Note You need to log in before you can comment on or make changes to this bug.