[qmtest] running qmtest on bash in cygwin...

[Stefan Seefeld]

Mark Mitchell wrote:
> Stefan --
> 
> Thanks for working on this!  This is a very good idea, and I appreciate you
> working it through.
> 
> 
>>class qm_install_data(install_data.install_data):
>>    """override install_data to generate a 'config' module
>>    containing the path to the data dir (and possibly more)"""
> 
> 
> Please use complete sentences (staring with a capital letter and ending with
> a period) in documentation.  This applies here and elsewhere.  (I am
> notoriously picky about this kind of thing, so I apologize in advance.)

ok.

>>classes=['classes.qmc',
> 
> 
> Is there a way to do this in an automated fashion?  I hate the idea of
> having to remember to update this list every time we add a new test class.
> How about just finding all the ".py" files in the "classes/" directory?

yeah, sure, that's possible (as you demonstrated in the other setup.py
file). I just usually prefer explicit over implicit, but I sure can live
with that.

Does the same apply for the other sets of files ? I.e., do you want (as
in the original setup.py file) to find all modules to install by looking
for __init__.py files ?

>>+try:
>>+    # this file was generated during the build
> 
> 
> It's helpful to be a bit more specific in comments; something like:
> 
>   # The config file is created during "make install" by setup.py.
> 
> ?

ok.

>>+    from qm.config import config, version
>>+    version_info = tuple(string.split(version, '.'))
>>+    """The version of QM as a tuple of (major, minor, release)."""
> 
> 
> Put single quotes around the tuple, i.e.:
> 
>    """The version of QM as a tuple '(major, minor, release)'."""
> 
> That makes it look right in HappyDoc.

ok.

>>+    # for now insert dummy values here so qmtest can still be
>>+    # built and installed the old way. To be removed eventually...
> 
> 
> We avoid "for now" or "FIXME" comments because they tend to live on longer
> than we'd like, and users see them and get worried.  So, I'd just say:
> 
>   # If qm.config was not available, we are running out of the source tree.

fine. Do you have any other means to remember open issues (a 'TODO' file
or something similar ?)

>>+    #print 'insert a meaningful error message here explaining'
>>+    #print 'the user not to run qmtest from the source tree'
>>+    #import sys
>>+    #sys.exit(-1)
> 
> 
> Remove this stuff.

ok.

>>     # Search the builtin directory, too.
>>+    # the old way...
>>     dirs.append(os.path.join(os.path.dirname(__file__), "classes"))
>>+    # ...and the new one
>>+    dirs.append(os.path.join(qm.config.data_dir, 'test', 'classes'))
> 
> 
> Those comments are confusing, if you don't have the whole source tree
> swapped into your brain.
> 
>   # When running from the source tree, we look for path relative to this
> file.
>   ... dirname(__file__)...
>   # In an installed version of QMTest, the config object tells us where to
> look.
>   ... qm.config.data_dir ...
> 
> And do we actually need to do this given that the config object generated
> when running from the source tree sets data_dir itself?

You are right, it's redundant. However, that brings up another point:
The file layout in the build tree is not the same as in the source tree.
Would you mind (eventually) refactoring the directory structure in the
source tree to better match the installation (for example take the 
qmtest.py script out of the modules, put resource files and even the 
builtin 'classes' into 'share/qm/test', etc.) ?

Of course, such a change should only be performed when the new build
system is ready so we don't have to adjust two build systems.

> Would you send me an SSH key so that we can get CVS write access set up for
> you?

here it is: http://www3.sympatico.ca/seefeld/ssh.txt

I'll wait and commit the corrected patch myself, then...

Regards,
		Stefan