FindRuby.cmake

[Michael Jansen]

On Monday 14 March 2011 22:23:54 Alexander Neundorf wrote:
> On Monday 14 March 2011, Alexander Neundorf wrote:
> > On Monday 14 March 2011, Michael Jansen wrote:
> ...
> 
> > 0002-Reorder-stuff-in-the-hope-it-is-more-readable-and-un.patch
> > I don't think this patch improves readability.
> > 
> > 0003-More-commenting.patch
> > IMHO this one doesn't really improve the comments.
> > 
> > -# if ruby > 1.8 is required or if ruby > 1.8 was found, search for the
> > config.h dir +#
> > +### FIND THE CONFIG.H FILE (IF > RUBY 1.8)
> > +#
> > IMO not better than before.
> > 
> > +#
> > +### CALL FindPackageHandleStandardArgs
> > +#
> > 
> >  INCLUDE(FindPackageHandleStandardArgs)
> >  SET(_RUBY_REQUIRED_VARS RUBY_EXECUTABLE RUBY_INCLUDE_DIR RUBY_LIBRARY)
> > 
> > This comment doesn't add any information.
> > 
> > And why are they all UPPERCASE ?
> 
> Or to put it in other words: I kind-of maintain (i.e. fix bugs from time to
> time) the FindRuby.cmake in CMake, and I don't feel like merging these two
> patches into cmake, since they do not really improve the file IMO.

The rational is that the script is more or less a about 200 whatever listing 
of cmake commands with some comments sprinkled inbetween. That what i call 
spaghetti code and refactor. In every language. In C++ i would factor it out 
into small helper functions with a description and one duty. In cmake i can't. 
So i try to put comments into the file that are easily visible distuingishable 
from code that explain the steps and try to minimize cross dependencies 
between the blocks.

I for one did only notice that RUBY_ARCH is not cached after that commits. I 
was only able to see at a glimpse which variable belongs to which installation 
are after those patches. And what other vars are set.

It is your decision but i would like to assure that i would have never 
finished the task without them. I btw. do not care the slightest bit about 
upper/lowercase or wording.  If you can improve anything feel free. The 
FindPackageHandleStandardArgs comment is there because i have no better 
explanation what this statement and the surrounding code does. Because i do 
not know what it does.

In the end it is your call. But always remember that not everyone understands 
250 lines of cmake code without any visible grouping or explanation what each 
part does.

Mike