Re: [PATCH 14/18] WTX-TCL support module

Joel Brobecker
2011-02-25T18:09:39Z

Thanks for the review!

(it's really late now, for me, but I wanted to give a brief answer)

> I guess this explains why the new code isn't part of
> --enable-targets=all. I am somewhat concerned that this will lead to
> bit-rot. I always build using this option in an attempt to avoid
> breaking things; but this code will not be included in that.
>
> I don't really see a way around it, though, unless you want to
> virtualize all the wtx calls.
>
> I guess I want us to be clear that build breakage for this is expected.

I think that's fair. I don't see what kind of virtualization you
are proposing that would help with that. Note that we link against
the TCL library provided by WindRiver. In all honesty, I didn't
really think about using a different TCL, since we didn't need to.

The good news is that, in the 5 or 6 years since I wrote the code,
I never experienced any breakage (that I can remember!)

I'll take care of all the other things you pointed out.

> Joel> + add_com ("tcl", class_obscure, tcl_command,
> Joel> + _("Evaluate the arguments with the TCL interpreter"));
>
> I am not super fond of a top-level command named "tcl".

I wasn't sure about that. I don't mind changing it. I thought that,
since it's another intepreter like Python, it might make sense to
put it in the top-level namespace. But I can either make it a subcommand
like "wtx tcl", or if you have another command name in mind, let me
know.

--
Joel