[Bug 516364] Review Request: xrdp - Open source remote desktop protocol (RDP) server

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.


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





--- Comment #2 from Mamoru Tasaka <mtasaka at ioa.s.u-tokyo.ac.jp>  2009-08-10 14:33:56 EDT ---
Some notes:

* License
  - As far as I checked the whole codes, the license tag
    should be "GPLv2+ with exceptions"

* -libs
  - Would you explain why you want to create -libs subpackage?

* SourceURL / Naming
  - For cvs based source, please follow:
    https://fedoraproject.org/wiki/Packaging/SourceURL#Using_Revision_Control
   
https://fedoraproject.org/wiki/Packaging/NamingGuidelines#PreReleasePackages

* Miscs
  - In comments or %changelog, please use %% instead of % to avoid
    macros expansion.

  - About Patch1, I would recommend to just include your new
    startwm.sh as new source (because it can be seen much easier).
  - For Patch0, maybe
    $ sed -i -e '/\[xrdp2\]/,$d' xrdp/xrdp.ini
    may be easier.

  - Please fix the following rpmlint(s)
---------------------------------------------------------------------
xrdp.i686: E: subsys-not-used /etc/rc.d/init.d/xrdp
xrdp-debuginfo.i686: W: spurious-executable-perm
/usr/src/debug/xrdp-0.5.0/keygen/keygen.c
---------------------------------------------------------------------

* Scriptlets
  - Calling /sbin/ldconfig is not needed (for -libs subpackage)
    because no libraries are installed under default library search
    paths (and so I wonder why -libs splitting is needed)

* Files
  (If you want -libs subpackage)
  - Documents like COPYING or so should belong to -libs subpackage
    because -libs subpackage can be installed without main package
    while main package cannot be installed without -libs subpackage

  - Same for the ownership of the directory %{_libdir}/xrdp

  - Please example what package should own the directory
    %_datadir/xrdp

  - Would you explain why you want to write "default" attribute like
    %attr(0755,root,root) or %attr(0644,root,root) explicitly?
    You can set these permissions at %install instead and remove
    these explicit %attr if you are unsure.

-- 
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.