whoohooh! A patch! Looks pretty sweet. Before we chuck it in, some
comments...
First of, I presume you have some tests (eg project.xml files and a
sample workspace) against which you run this code. Could you submit
those too?
> + def __init__(self, project, target, buildfile="maven.xml",
basedir=None, properties=None):
Isn't using 'maven.xml' mandatory? I think you can set the POM but not
the buildfile...
> + #command line
> + args = ["maven jar "]
If I'm not mistaken args should be one-entry per array, eg
["maven", "jar"]
also, the "target" defined in the model as above is probably the maven
"goal" to use instead of the hardcoded "jar"?
Leo Simons wrote:
> Index: pygump/python/gump/engine/map.py
could map.py go into gump/util or gump/plugins/java somewhere? Also,
it'd be cool if this could be a config file of some sort that is read
in, perhaps a format as simple as
avalon-framework avalon-framework-api
axis ws-axis
Note that the 'gump' plugin for maven has many more of these mappings
that would be nice to steal.
> Index: pygump/python/gump/engine/mavenizer.py
> ===================================================================
> + if maven_node:
> + #parse DOM creating new gumped-maven file
> + #then return file location to be used instead
> + #of original maven file
> + gumped_maven_href = _parse_maven_file( maven_node,
> download_func,
> get_vfs )
> + return gumped_maven_href
> + else:
> + return 'broken'
you want to 'raise' an exception here.
> +def _parse_maven_file( project, download_func, get_vfs ):
this looks pretty comprehensive!
> + mavenfile = open("%s\\%s" % (home_dir, filename), "w")
^^^ use os.path.seperator (or whatever its
called)
however, using the StringIO module would probably avoid the need for
using a file at all, which would rock even more.
> Index: pygump/python/gump/config.py
> ===================================================================
> --- pygump/python/gump/config.py (revision 233131)
> +++ pygump/python/gump/config.py (working copy)
> @@ -105,6 +105,7 @@
> log.info("Not running updates! (pass --do-updates to enable
them)")
>
> if config.do_build:
> + from gump.model import Maven
> from gump.model import Ant
> from gump.model import Script
looks like a TAB character in there. Seems there's several more in your
patch. Please get rid of those and just use spaces :)
> Index: pygump/python/gump/util/executor.py
> ===================================================================
> --- pygump/python/gump/util/executor.py (revision 233131)
> +++ pygump/python/gump/util/executor.py (working copy)
> @@ -38,7 +38,7 @@
> __copyright__ = "Copyright (c) 2004-2005 The Apache Software Foundation"
> __license__ = "http://www.apache.org/licenses/LICENSE-2.0";
>
> -import sys
> +import sys, os
huh, what? Why? Seems there's a few "loose ends" in the patch...
> Index: pygump/python/gump/util/io.py
> ===================================================================
> --- pygump/python/gump/util/io.py (revision 233131)
> +++ pygump/python/gump/util/io.py (working copy)
> @@ -99,6 +99,9 @@
> self.cachedir = os.path.normpath(os.path.abspath(cachedir))
> else:
> self.cachedir = None
> +
> + def return_path(self):
> + return self.filesystem_root
feel free to just use the property directly, eg replace calls to
myvfs.return_path() with simply
myvfs.filesystem_root
instead of writing "accessors". Sun did the world a lot of harm by
inventing getters/setters for java. Python certainly doesn't need them.
cheers,
Leo
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
