Bug 427618 - Review Request: nvclock - Utility that allows users to overclock NVIDIA based video cards
Summary: Review Request: nvclock - Utility that allows users to overclock NVIDIA based...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Dominik 'Rathann' Mierzejewski
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-01-05 14:08 UTC by Adel Gadllah
Modified: 2008-01-07 15:32 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-01-07 15:32:55 UTC
Type: ---
Embargoed:
dominik: fedora-review+
kevin: fedora-cvs+


Attachments (Terms of Use)

Description Adel Gadllah 2008-01-05 14:08:00 UTC
Spec URL: http://tgmweb.at/gadllah/nvclock.spec
SRPM URL: http://tgmweb.at/gadllah/nvclock-0.8-0.1.b3.fc8.src.rpm
Description: 
Nvclock allows you to tweak your Nvidia card under Linux.
It features overclocking, hardware monitoring, tweaking
of OpenGL settings, and much more.

Comment 1 Dominik 'Rathann' Mierzejewski 2008-01-05 14:40:55 UTC
Taking review.

Comment 2 Dominik 'Rathann' Mierzejewski 2008-01-05 17:34:31 UTC
rpmlint output clean:
$ rpmlint /var/lib/mock//fedora-development-i386/result
nvclock-gtk.i386: W: no-documentation

Source md5 matches upstream:
eab0a35a79f0165310be759b043deedb  nvclock0.8b3.tar.gz

Builds in mock: devel/i386, F-7/x86_64

Problems:
1. License field says GPLv2+, but
src/libc_wrapper.c has MIT license (GPLv2+ compatible)
src/backend/i2c.c and nv40.c have some additional (apart from the GPL) clauses -
can you verify that they don't impose any restrictions over GPL terms?

2. This part could use a little more spacing for legibility:
[...]
%package gtk
Summary: Nvclock gtk frontend
Group: Applications/System
%description gtk
This package contains the nvclock gui.
[...]

3. make is called without %{?_smp_mflag} and no comment is present explaining why.

4. Since the icon you add is 48x48, why not put it in
%{_datadir}/icons/hicolor/48x48/apps/
?
Of course, then you need to add
%post gtk
touch --no-create %{_datadir}/icons/hicolor
if [ -x %{_bindir}/gtk-update-icon-cache ]; then
  %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
fi

%postun gtk
touch --no-create %{_datadir}/icons/hicolor
if [ -x %{_bindir}/gtk-update-icon-cache ]; then
  %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
fi

5. Why aren't you building the QT interface? AFAICT it builds and works just as
well as the GTK interface.


Comment 3 Adel Gadllah 2008-01-05 18:38:08 UTC
(In reply to comment #2)

> Problems:
> 1. License field says GPLv2+, but
> src/libc_wrapper.c has MIT license (GPLv2+ compatible)
so should I add GPLv2+ and MIT ?
> src/backend/i2c.c and nv40.c have some additional (apart from the GPL) clauses -
> can you verify that they don't impose any restrictions over GPL terms?

I asked upstream and got this response 
"it are just some pieces from xfree86, the same stuff that is now in the xorg nv
driver"

> 2. This part could use a little more spacing for legibility:
> [...]
> %package gtk
> Summary: Nvclock gtk frontend
> Group: Applications/System
> %description gtk
> This package contains the nvclock gui.
> [...]

ok

> 3. make is called without %{?_smp_mflag} and no comment is present explaining why.

because the build fails; have not yet investigated why .. will add a comment.

> 4. Since the icon you add is 48x48, why not put it in
> %{_datadir}/icons/hicolor/48x48/apps/
> ?
> Of course, then you need to add
> %post gtk
> touch --no-create %{_datadir}/icons/hicolor
> if [ -x %{_bindir}/gtk-update-icon-cache ]; then
>   %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
> fi
> 
> %postun gtk
> touch --no-create %{_datadir}/icons/hicolor
> if [ -x %{_bindir}/gtk-update-icon-cache ]; then
>   %{_bindir}/gtk-update-icon-cache --quiet %{_datadir}/icons/hicolor || :
> fi

OK, will do.

> 5. Why aren't you building the QT interface? AFAICT it builds and works just as
> well as the GTK interface.
> 

Because its unmaintained and upstream is planning to drop it completly. 



Comment 5 Dominik 'Rathann' Mierzejewski 2008-01-06 00:42:44 UTC
Well then, everything seems to be in order. No further comments.

APPROVED.

Comment 6 Adel Gadllah 2008-01-06 01:23:19 UTC
(In reply to comment #5)
> Well then, everything seems to be in order. No further comments.
> 
> APPROVED.

Ok, thx for the review!

==============

New Package CVS Request
=======================
Package Name: nvclock
Short Description: Utility that allows users to overclock NVIDIA based video cards
Owners: drago01
Branches: F-7 F-8
Cvsextras Commits: yes


Comment 7 Kevin Fenzi 2008-01-06 17:39:39 UTC
cvs done.

Comment 8 Dominik 'Rathann' Mierzejewski 2008-01-07 15:32:30 UTC
Reopening to reassign to myself.


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