Dominique Devienne wrote: >On 8/9/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: >> Steve Loughran said: >> >2. I'd put the nested commands into a <sequence>, the way we did in >> ><macrodef>. This makes it clear it is sequential, and it leaves room to >> >add new things alongside <localtunnel>. > >> I'd like to discuss further your suggestion of incorporating <sequence> >> (<sequential>, according to 1.7.0 doc??) into sshsession to hold the >> nested tasks. Please explain further why this is prefereable. > >Forward compatibility. Right now you allow: > ><sshsession ...> ><LocalTunnel ... /> >any tasks... ></sshsession> > >So someone somewhere use <foo/> in "any tasks...", and later on you >want to enhance <sshsession> with a new <foo/> element. That someone's >build would now break with your new enhanced version. > >Contrast this with this model: > ><sshsession ...> ><LocalTunnel ... /> ><sequential> >any tasks... ></sequential> ></sshsession> > >Now you are free to add new elements directly under <sshsession> >without potentially breaking any builds. > >Code-wise, composing a Sequential instance is better/simpler IMHO than >inheriting TaskContainer (or Sequential).
I don't know about simpler code, but certainly not much harder, and probably better, for the reason you cite. I see your argument for avoiding conflicting/ambiguous <foo/> elements, and after looking over MacroDef, I see that the <Sequential> there is built by MacroDef, not the same as the <Sequential> Task, so it is still free to process some future <sshremotecommand/> (renamed from my earlier <command/> suggestion, to eliminate the future conflict issue you raise) nested along with other tasks *within* <Sequential>. > >An alternative in line with your proposed extension is to move the >inheritance away from <sshsession> and into <localtunnel> and ><remotetunnel>, to allow: > ><sshsession ...> ><LocalTunnel ...> >any tasks... ></LocalTunnel> ><RemoteTunnel ...> >any tasks... ></RemoteTunnel> ></sshsession> > >Here as well, there's no possible conflict with a user script using an >element name you'd want to add to <sshsession> itself. I'm not keen on this. It restricts you to having only one tunnel at a time. I think I'd like to establish all tunnels up front, then any of the nested tasks can use any of the tunnels, in any order. There is already logic checking that you aren't defining conflicting tunnels (unique lport on localtunnel and unique rport on remotetunnel) with the expectation that you may have multiple <localtunnel/> and/or <remotetunnel/> elements for a single <sshsession>. > >I hope this helps. --DD > Yes, thanks.
