[Bug 868931] Review Request: sshuttle - Transparent Proxy VPN

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Product: Fedora
https://bugzilla.redhat.com/show_bug.cgi?id=868931

Tom "spot" Callaway <tcallawa@xxxxxxxxxx> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tcallawa@xxxxxxxxxx
           Assignee|nobody@xxxxxxxxxxxxxxxxx    |tcallawa@xxxxxxxxxx
              Flags|                            |fedora-review+

--- Comment #14 from Tom "spot" Callaway <tcallawa@xxxxxxxxxx> ---
= Review =

Good:

- rpmlint checks return:
sshuttle.src:2: W: macro-in-comment %adgit
sshuttle.src: W: invalid-url Source0: sshuttle-0-20120810git9ce2fa0.tar.gz
sshuttle.noarch: E: incorrect-fsf-address /usr/share/doc/sshuttle-0/LICENSE
2 packages and 0 specfiles checked; 1 errors, 2 warnings.

Since you've informed upstream about the FSF address issue, and the Source0 is
properly documented in comments, and the macro-in-comment warning is a false
positive, all of these are safe to ignore.

- package meets naming guidelines
- package meets packaging guidelines
- license (LGPLv2+) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream (manual diff matches upstream checkout)
- package compiles on devel (noarch)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file 

Minor:

You have %doc %{_mandir}/man8/sshuttle.8.gz. The %doc here is redundant,
everything in %{_mandir} is always automatically %doc. Just drop that before
you do builds. :)

I also usually prefer that files which end up in the package are not generated
within the spec, but rather, included as separate Source files. This isn't
strictly against the guidelines, since there are cases where you need to define
paths at buildtime (usually involving %{_libdir}), but in this case it really
isn't necessary. That said, I won't make you change this behavior, it is up to
you how you wish to do it.

Since you seem to already be sponsored, this package is APPROVED.

-- 
You are receiving this mail because:
You are on the CC list for the bug.
Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=vLkjCcunJV&a=cc_unsubscribe
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review



[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]