[Bug 510734] Review Request: x11vnc - VNC server for the current X11 session

Wed Aug 26 14:14:08 UTC 2009

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=510734

--- Comment #53 from Pavel Alexeev (aka Pahan-Hubbitus) <pahan at hubbitus.info>  2009-08-26 10:14:02 EDT ---
(In reply to comment #52)
> 2. binary files - TODO
> The upstream package ships some pre-compiled java binaries:
> x11vnc-0.9.8/classes/VncViewer.jar
> x11vnc-0.9.8/classes/ssl/VncViewer.jar
> x11vnc-0.9.8/classes/ssl/SignedVncViewer.jar
> x11vnc-0.9.8/classes/ssl/UltraViewerSSL.jar
> x11vnc-0.9.8/classes/ssl/SignedUltraViewerSSL.jar
> 
> The jar files can be re-generated using the shipped sources (GPLv2).
> However, as Tom Callaway pointed out, only if we are 100% sure about the
> actual license of the binary files we may distribute it.
> I have checked their web page as well as the various README files in the
> package and I could not find any specific information regarding the status
> of the binaries. This means, that I'm not 100% sure about it and so I think
> we should re-package it:
As I already say before, Karl Runge (upstream developer) answer me what there
no non-free code, and all sources also included there. I have not check it. But
have we any foundation to do not trust him??

> http://fedoraproject.org/wiki/Packaging:SourceURL#When_Upstream_uses_Prohibited_Code
If you very want, off course I may repack it.

> However, if Tom or somebody else from the FE-LEGAL team gives their explicit
> permission for redistribution I would be happily accept this. ;-)
Tom lifting FE-legal early :)

> * specfile in American English and legible: TODO
> - indentation: I know this may sound like nitpicking, however I'm still
> convinced that the spec files should be written in such a way that it can be
> easily read by any other user with any program. It should not be necessary to
> guess the tab width for each spec file. ;-) I think using some kind of standard
> for things like this will certainly help that all Fedora package maintainers
> can easily work together.
There was discussion on this theme -
http://thread.gmane.org/gmane.linux.redhat.fedora.extras.packaging/6214/focus=6224
And accordingly it proposed draft guidelines change -
https://fedoraproject.org/wiki/PackagingDrafts/Tabs

So, there many people argue with your arguments. So, I think until it is only
draft and FESCO do not approve that, it can't be as required item.

> - typo: pathes -> paths
> - wording: IMHO "performant" isn't an Enlish word, probably just use "fast"
Sure? http://dictionary.reference.com/browse/performant
But if you want, I replace it by productive. Is it ok?

> - spelling: "prebuild clients" -> "prebuilt clients"
> - spelling: "acording" -> "according"
> - spelling: "macroses" -> "macros"
There I don't known (replaced as you say). Macros have not multiple form at
all?
> - spelling: "arount" -> "around"
Thank you. All fixed.

> - indentation in %prep:
> The small code snippet to do the conversion into UTF-8 is not indented
> correctly. The begin ("for file....") and the end ("done") of the for loop
> should be one tab more to the left than the body of the for loop.
Again...
I'm change it as you like see it for only do not start new long flame-war.
But this is very similar to tab-width space. In Fedora we even not have (as I
known) some recommended "coding standards" or similar
documents/guidelines/policies. So, than it can't be error at all!

> 
> - please order the patches by number
Ok.
> 
> * %description: TODO (minor)
> - Although "x11vnc" is a name, I would start it with a capital "C" at the
> beginning of the sentence.
Capital "X" I think? I'm unsure. BTW, I reorder sentence, please see.
> - In the second sentence I think the comma can be omitted.
Ok.

> * Patch0: TODO
> During the review I was wondering that x11vnc is neither linked against the
> lzo library nor the minilzo library. Indeed since we neither build or use
> libvncclient nor libvncserver from the x11vnc package, there is no need at all
> to take care of lzo here.
> Just to be on the save side I would still delete the minilzo.[ch] in the
> spec file, but I think the lzo patch as well as the usage of __ln_s/__sed can
> be omitted.
Yes, you are right. Today, after linking with system libvncserver.

> * Compilation: TODO
> - the first 2 lines which exports the CFLAGS and the LDFLAGS should not be
> necessary
Ok.

> * BuildRequires: TODO
> - most likely lzo-devel can be omitted
Ok.

> * packaged files and directories: TODO
> It looks like that /usr/share/x11vnc is packaged although it is empty.
> Please don't package it.
Ok, excluded.

http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc-0.9.8-10.fc11.src.rpm
http://hubbitus.net.ru/rpm/Fedora11/x11vnc/x11vnc.spec

-- 
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.