Bug 199627 - Review Request: kooldock - dock for KDE with great visual effects and enhancements
Review Request: kooldock - dock for KDE with great visual effects and enhance...
Status: CLOSED NEXTRELEASE
Product: Fedora
Classification: Fedora
Component: Package Review (Show other bugs)
rawhide
All Linux
medium Severity medium
: ---
: ---
Assigned To: Rex Dieter
Fedora Package Reviews List
:
Depends On:
Blocks: FE-ACCEPT
  Show dependency treegraph
 
Reported: 2006-07-20 16:24 EDT by Michał Bentkowski
Modified: 2007-11-30 17:11 EST (History)
0 users

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2006-09-04 13:30:29 EDT
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:


Attachments (Terms of Use)
Patch to fix rpmlint warning on the srpm. (1.44 KB, patch)
2006-07-21 16:53 EDT, Devrim GUNDUZ
no flags Details | Diff

  None (edit)
Description Michał Bentkowski 2006-07-20 16:24:12 EDT
Spec URL: http://ecik.zspswidwin.pl/kooldock/kooldock.spec
SRPM URL: http://ecik.zspswidwin.pl/kooldock/kooldock-0.3-1.20060720cvs.src.rpm
Description: This is one of my first packages, and I'm looking for a sponsor.
KoolDock is a KDE project that aims to have a cool dock
for KDE with great visual effects and enhancements.
Some of it features are:
- Display quick launchers to your favourite apps
- A builtin task bar
- Pager and clock. (Not done yet)
- Smooth zooming effect (like Apple's OS X dock)
- Transparent Background.

Comments:
It uses my own cvs snapshot source, because kooldock site is down, and there
is no option to download the source. Probably, CVS snapshot will never be
updated (the last update is 2 years old) and package is probably dead but...
I use it every day and it looks really cool ;) so I think it would be good if 
the latest version will turn up in Extras.
Package should be easy to review, because rpmlint doesn't show anything and mock build completes successfully ;-)
Comment 1 Devrim GUNDUZ 2006-07-21 16:53:01 EDT
Created attachment 132846 [details]
Patch to fix rpmlint warning on the srpm.

Patch fixes the following warning:
W: kooldock mixed-use-of-spaces-and-tabs
Comment 2 Devrim GUNDUZ 2006-07-21 16:57:36 EDT
BTW, the package builds cleanly on FC5 and rpmlint gives no errors for the rpm.
Comment 3 Michał Bentkowski 2006-07-21 17:03:43 EDT
(In reply to comment #2)
> BTW, the package builds cleanly on FC5 and rpmlint gives no errors for the 
rpm.

Now I know, why I haven't noticed mixed-use-of-spaces-and-tabs warning...
Because I always use it to rpm, never to srpm.
Comment 4 Michał Bentkowski 2006-07-21 17:10:23 EDT
I've sent new inital release of SPEC and SRPM files:
Spec URL: http://ecik.zspswidwin.pl/kooldock/kooldock.spec
SRPM URL: http://ecik.zspswidwin.pl/kooldock/kooldock-0.3-1.20060720cvs.src.rpm
Comment 5 Jason Tibbitts 2006-07-21 19:36:06 EDT
Removing FE-NEEDSPONSOR as Michał has been sponsored.
Comment 6 Devrim GUNDUZ 2006-07-23 16:09:22 EDT
A few more comments:
* I think you should change Source0 to use %{name} instead of kooldock. 
* The same is valid for the line beginning with %setup
* You should use cvs_date macro in Source0
* You should add  %{?smp_flags} to make in %prep

That is all I can see now. Per Review Guidelines, I tried to test everything
except mock build.

Regards, Devrim
Comment 8 Rex Dieter 2006-07-26 08:03:26 EDT
I can review this...
Comment 9 Rex Dieter 2006-07-26 08:06:07 EDT
Grr. http://ktown.kde.cl/kooldock/ seems unreachable atm, I'll try again 
later.
Comment 10 Michał Bentkowski 2006-07-26 08:09:05 EDT
Well, I don't think this site will be available at any time. It looks dead for
a long time.
Comment 11 Rex Dieter 2006-07-26 08:19:21 EDT
1.  %prep: the rm should come *after* the %setup, else, there's nothing 
there/unpackage yet to delete.

2.  drop hard-coded
Requires: kdelibs

3.  If this is a cvs snapshot prerelease, Release tag should be prefixed with 
0. so use something like this instead:
Release: 0.2.%{cvs_date}cvs%{?dist}

4.  The bit in %build
export QTLIB...
isn't needed anymore to workaround qt bug(s).  The bugs have been fixed.

5.  Drop 
%doc INSTALL
no need for INSTALL instructions here. (:

6.  Since you're installing icons, you should include scriptlets:
%post
touch --no-create %{_datadir}/icons/crystalsvg || :

%postun
touch --no-create %{_datadir}/icons/crystalsvg || :

7. You probably ought to change
%defattr(0644,root,root,0755)
to
%defattr(-,root,root,-)
then you won't need the %attr bits for:
%attr(0755,root,root)%{_bindir}/*

8. Upstream source.  If you're going to be using a cvs checkout, please provide 
a script to (re)generate said source tarball.
Comment 12 Rex Dieter 2006-07-26 08:20:10 EDT
>Well, I don't think this site will be available at any time. It looks dead for
>a long time.

Then I suggest you find a better/working site use for
URL:
Comment 13 Michał Bentkowski 2006-07-26 08:33:07 EDT
(In reply to comment #11)

> 3.  If this is a cvs snapshot prerelease, Release tag should be prefixed with 
> 0. so use something like this instead:
> Release: 0.2.%{cvs_date}cvs%{?dist}

This is POSTrelease.

> 8. Upstream source.  If you're going to be using a cvs checkout, please 
provide 
> a script to (re)generate said source tarball.

Could you explain me what do you exactly mean?

URLs:
SPEC URL: http://ecik.zspswidwin.pl/kooldock/kooldock.spec
SRPM URL: http://ecik.zspswidwin.pl/kooldock/kooldock-0.3-3.20060720cvs.src.rpm
Comment 14 Rex Dieter 2006-07-26 08:37:18 EDT
> This is POSTrelease.

Then never mind, no need to use a 0. prefex.

>> a script to (re)generate said source tarball.

> Could you explain me what do you exactly mean?

Include
Source1: kooldock-cvs_checkout.sh
that contains (something like):
----------------------------------
#!/bin/bash
MODULE=$(basename $0 -cvs_checkout.sh)
DATE=$(date +%Y%m%d)

set -x
rm -rf $MODULE
cvs -z3 -d:pserver:anonymous@${MODULE}.cvs.sourceforge.net:/cvsroot/$MODULE co
-P $MODULE
tar cjf $MODULE-${DATE}cvs.tar.bz2 $MODULE
rm -rf $MODULE
--------------------------
Comment 15 Michał Bentkowski 2006-07-26 08:41:30 EDT
There is no need to it. As I wrote in my first comment, project is dead
and will never be continued. The last change in CVS is two years old.
I have done one cvs snapshot on my server and this is enough.
Comment 16 Rex Dieter 2006-07-26 08:48:41 EDT
> As I wrote in my first comment, project is dead
> and will never be continued.

My bad for missing that, sorry.

With upstream project being dead, it's ill-advised to bring this into Extras. 
I'm not willing to approve this. 

Comment 17 Michał Bentkowski 2006-07-26 09:22:36 EDT
(In reply to comment #16)
> With upstream project being dead, it's ill-advised to bring this into Extras. 
> I'm not willing to approve this. 

I reported this package, because despite of its dead it still looks nice and 
works nice and, for me, it is the greatest docker for KDE.
Comment 18 José Matos 2006-07-26 09:37:25 EDT
In reply to #17

>I reported this package, because despite of its dead it still looks nice and 
>works nice and, for me, it is the greatest docker for KDE.

Then the solution is obvious, you should become the upstream source maintainer 
for this package.
Comment 19 Michał Bentkowski 2006-07-26 09:40:32 EDT
(In reply to comment #18)
> Then the solution is obvious, you should become the upstream source 
maintainer 
> for this package.

I am. I've done my own cvs snapshot and this package uses it.
Comment 20 Rex Dieter 2006-08-17 15:28:29 EDT
Fair enough, game on.
Comment 21 Rex Dieter 2006-08-17 16:04:39 EDT
Michal, the package looks in pretty good shape, now for some testing...
Comment 22 Michał Bentkowski 2006-08-17 16:08:32 EDT
Why have you added FE-NEEDSPONSOR block? I'm already sponsored and I'm owner
of kadu and python-mutagen packages.
Comment 23 Rex Dieter 2006-08-17 16:32:06 EDT
Because I'm lame and read comment #1 that said you were looking for a sponsor, 
and *assumed*... (:
Comment 24 Michał Bentkowski 2006-08-17 16:39:16 EDT
(In reply to comment #23)
> Because I'm lame and read comment #1 that said you were looking for a 
sponsor, 
> and *assumed*... (:

Never mind ;)
So, what with approvement this package to extras?
Comment 25 Rex Dieter 2006-08-17 16:54:43 EDT
looks good, APPROVED.
Comment 26 Rex Dieter 2006-08-22 08:45:03 EDT
Don't forget to close this bug once you've imported and submitted a build.
Comment 27 Michał Bentkowski 2006-09-04 13:30:29 EDT
Closing it.

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