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

Zbigniew JÄ™drzejewski-Szmek <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |POST
              Flags|fedora-review?              |fedora-review+



--- Comment #9 from Zbigniew JÄ™drzejewski-Szmek <[email protected]> ---
> Seems that it was written to be started by hands but not by any init system. 
> But I think that systemd service is a good option anyway.
Yes, this service file is better than nothing. I'm just saying that this
is something that would be nice to fix in the long run.

> There are several settings that can be specified only as command line options
> (address to listen on etc.). That's why I prefer to allow setting these 
> options via file in /etc/default instead forcing users to use defaults.
This is not a packaging issue, but an upstream issue. But vrpn already has a
config file, so it should simply read whatever options it need itself. Having a
second config file is just a workaround for a stupid deficiency in the server.
If you need to provide configuration options on the command-line, a systemd
unit drop-in override is probably a better option.

But that somewhat matter of opinion, so I'll just get on with the review ;)

+ latest version
+ license is OK (GPLv3+)
- %license should be used for README.Legal
  README doesn't have to be packaged, since it just refers to README.Legal

- %python_provide macro should be used
[https://fedoraproject.org/wiki/Packaging:Python#The_.25python_provide_macro]
+ provides/requires look OK otherwise
+ scriptlets look sane
+ builds and installs OK

Package is APPROVED. Please fix the license stuff and python provides.


You might consider defining %global _description to hold the repetead part of
description. One trick is to do something like this:
%global _description The Virtual........................................... \
..............................................................    \
................................................................  \
.................................................................
(I.e. wrap the text to 80 columns but then merge the first row with %global.)
This way you avoid an empty line in %description. I haven't found a nicer
way to do this.

You can also replace Requires*: systemd lines with %systemd_requires. FPC just
removed the ban a few days ago and it's more consise.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
[email protected]
https://admin.fedoraproject.org/mailman/listinfo/package-review

Reply via email to