Fwd: Re: [Lcdproc] Re: VLSYS L.I.S 2005 Driver written

[Peter Marschall]

Hi Daryl,

On Tuesday, 8. May 2007 02:01, Daryl F wrote:
> Here's the patch for the VLSYS L.I.S driver. It is for lcdproc-0.5.1.
> The code isn't really pretty. I had to do some guessing since VLSYS
> won't reveal the inner workings. But it works.

Thanks for the patch.

I have had a quick look at it, which made me consider it as not 
yet ready for inclusion into the LCDproc CVS.

Here are my reasons:
- the patch is based against 0.5.1
  Please re-base it against LCDproc CVS available at
  http://lcdproc.sourceforge.net/nightly/lcdproc-CVS-current.tar.gz
- it contains whole files (which are newer in CVS than in your version)
  Please reduce the patch to the changes only.
- there are issues with it posted on the mailing list
  See paul_c's and Todd Luliak's posts.
  Please try to address the issues mentioned.
  It is not acceptable for a patch to break compilation of LCdproc
  (and pleae do not expect the maintainer [who does not have
  the hardware] why).
- It lacks documentation completely
  Please add doxygen documentation to the source (see the
  IOWarrior or CFontzPacket drivers as an example) and
  add the other missing parts (see
  http://lcdproc.sourceforge.net/docs/current-dev.html#documentation
  what is expected)
Once these issues are addressed, I'll happily commit the drivr to CVS.

This may sound a bit harsh, but it is not ment that way.
It is only meant as a remnider what is expected of an
LCDproc driver: not only compile-able and working code, 
but also documentation accompanying it, so that users 
that obviously do not have such in-depth knowledge 
about the driver internals as you, the author,
can get the driver to run easily.

Thanks
Peter


-- 
Peter Marschall
peter@adpm.de