Hi
Your mail was very insightful! I'm going to focus on a few points and
will probably get back on other parts later.
jerome lacoste wrote:
[snip]
There's a certain legacy in the current commons-exec and it shows a
lot. The main issue is the following: commons-exec as today was
written to solve a particular problem within ant and not as a reusable
library. In particular, reusing it forces any client to reuse many
classes that a client should not necessarily care about. In comparison
ProcessBuilder (SDK 5.0) (which solves only a small subset of what
commons-exec solves), is much cleaner in its approach. It relies much
more on SDK classes: no need for a CommandLine class, no need for an
Environment class, etc...
Agreed. Like you, I think we can do a lot of work on getting exec into a
cleaner state by refactoring the current code.
[snip]
Vision & architecture:
---------------------------
- Execute (or Executor?): Flexible, we use it to tie together the
various concepts in the execute() call (stream management, process
management, process building, etc...)
- Exec (or ??) makes it easy to use Execute/or.
I think we should aim for removing Exec and instead focus on getting the
other classes, particulary Execute, easy enough to use without having a
helper class like Exec.
Detailed changes:
-------------------------
EnvironmentVariable. Too bad there's no real interface for key/value
container in SDK (apart from Map.Entry whose definition makes me want
to avoid using it outside of a Map). Otherwise I would have scrapped
it...
I think we can scrap it anyways.
Environment. 1st remove Process Environment out. It's got nothing to
do in there. Environment becomes mainly practical for clients who
already use the EnvironmentVariable as for internal building of the
ProcessEnvironment class. So let's make it a Map so that users don't
need it to use the other classes. Also make it map key->value instead
of key->(key, value)
Agreed.
ProcessEnvironment: We don't need a real class. What is the most
needed is a getenv() call that builds an Environment (like
System.getenv()). Could maybe just return a Map. But I don't see an
issue with returning the Environment itself which maybe helps us. We
see that as implementation goes.
Agreed. Where would you like to put the method for retriving the current
environment?
[snip]
We regard to my other patches, they can be discarded if we go for a
rewrite (through refactorings). Trygve: in particular the serializable
patch is not critical. Was just there to maintain the functionality of
the old API, as I thought it as critical as that time I started
looking at the project.
Would it be possible to break your refactorings down into a set of
patches that each fix on of the issues you outlined above?
I don't mind this process taking several months as long as there's a
good feedback from the potential users. I am not in a hurry to finish,
just in a hurry to get started :)
Again, agreed :-)
/niklas
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
