[Bug 852893] Review Request: ddccontrol - Control your monitor by software using the DDC/CI protocol

[bugzilla at redhat.com]

https://bugzilla.redhat.com/show_bug.cgi?id=852893

Jaroslav Škarvada <jskarvad at redhat.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
              Flags|needinfo?(jskarvad at redhat.c |
                   |om)                         |

--- Comment #2 from Jaroslav Škarvada <jskarvad at redhat.com> ---
(In reply to comment #1)
Thanks for the review, new files:
Spec URL: http://fedorapeople.org/~jskarvad/ddccontrol/ddccontrol.spec
SRPM URL:
http://fedorapeople.org/~jskarvad/ddccontrol/ddccontrol-0.4.2-2.fc17.src.rpm

> [!]: MUST Package contains desktop file if it is a GUI application.
> 
>      The desktop file is in a wrong subpackage. It should be in -gtk
> subpackage.
> 
Fixed.

> [!]: MUST Package complies to the Packaging Guidelines
> 
>      Why are the manual pages gzipped manually? Isn't this performed by
> rpmbuild?
> 

Fixed.

>      Is configuration file for autoloading really needed? If it is, you
> should relocate the configuration file to
> /usr/lib/modules-load.d/%{name}.conf.
> 
It is there to improve user experience - in most cases I2C is used for DDC
(even with binary blob drivers). The presence of this module should be harmless
and low footprint. Moved to /usr/lib/modules-load.d/%{name}.conf.

> [!]: MUST Requires correct, justified where necessary.
> 
>      Subpackage gtk requires the main package twice, with ISA and without,
> why?
> 
Fixed.

> [!]: MUST Rpmlint is run on all rpms the build produces.
> 
>      Please, justify: W: unused-direct-shlib-dependency
> 
Fixed.

> [x]: MUST Sources used to build the package match the upstream source, as
>      provided in the spec URL.
> [!]: MUST Spec file is legible and written in American English.
> 
>      I would change "buttons in front of the monitor" to "buttons on the
> monitor".
> 
To be even more generic used "monitor HW controls"

> [!]: SHOULD SourceX / PatchY prefixed with %{name}.
>      Note: Source1 (modules-autoload.conf)
>
Couldn't find this requirement in Review guidelines nor Packaging guidelines,
but fixed.

> ddccontrol.x86_64: E: incorrect-fsf-address
> /usr/share/doc/ddccontrol-0.4.2/COPYING
>
Problem with incorrect FSF snail mail address reported upstream.

New warnings appear:
ddccontrol.spec:48: E: hardcoded-library-path in
%{_prefix}/lib/modules-load.d/%{name}.conf
ddccontrol.spec:96: E: hardcoded-library-path in
%{_prefix}/lib/modules-load.d/%{name}.conf

Probably false positives.

-- 
You are receiving this mail because:
You are on the CC list for the bug.