HI Wing Yew,
I modified the code to make sure that backward compatibility is not messed up,
by removing the polymorphism. So this should no longer be a concern.
Hari
--
Hari Shreedharan
On Wednesday, January 30, 2013 at 5:57 PM, Wing Yew Poon wrote:
> I don't see any concern as long as
>
> shell.exec("cd /some/dir", "tar xvf some.tar", "hadoop fs -put
> contents contents");
>
> calls the exec without the int; while
>
> shell.exec(10000, "/some/dir/some.sh (http://some.sh)");
>
> calls the exec with the int.
> - Wing Yew
>
> On Wed, Jan 30, 2013 at 4:54 PM, Mark Grover
> <[email protected] (mailto:[email protected])> wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/9166/#review15912
> > -----------------------------------------------------------
> >
> >
> >
> > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
> > <https://reviews.apache.org/r/9166/#comment34193>
> >
> > Hi Hari, this is when java/groovy starts to get hairy for me so I wrote a
> > test in groovy. Here is how it looks:
> > private void test(int x, Object... args){
> > System.out.println("with x")
> > }
> >
> > private void test(Object... args){
> > System.out.println("without x")
> > }
> > test(1,2,3)
> > test('a','b','c')
> > test("abc")
> > test(1)
> > test(1, 'a')
> >
> > The output is:
> > with x
> > without x
> > without x
> > with x
> > with x
> >
> > Conclusion: If the first argument of the call is int, the method with 1st
> > int argument gets called (i.e. the signature you are adding) directly,
> > bypassing the existing method that just takes Object...
> >
> > Do you see any concerns here?
> >
> > 1. Do you think those concerns may be resolved by making the newly
> > signature private?
> > 2. Even better, do you think there is a way to accomodate the additional
> > timeout as a part of the existing arguments parameter without changing the
> > method signature?
> >
> >
> > - Mark Grover
> >
> >
> > On Jan. 30, 2013, 8:35 p.m., Hari Shreedharan wrote:
> > >
> > > -----------------------------------------------------------
> > > This is an automatically generated e-mail. To reply, visit:
> > > https://reviews.apache.org/r/9166/
> > > -----------------------------------------------------------
> > >
> > > (Updated Jan. 30, 2013, 8:35 p.m.)
> > >
> > >
> > > Review request for bigtop and Roman Shaposhnik.
> > >
> > >
> > > Description
> > > -------
> > >
> > > Added new methods which can execute a script for a specified timeout and
> > > can also execute in the background.
> > >
> > >
> > > This addresses bug BIGTOP-835.
> > > https://issues.apache.org/jira/browse/BIGTOP-835
> > >
> > >
> > > Diffs
> > > -----
> > >
> > > bigtop-test-framework/src/main/groovy/org/apache/bigtop/itest/shell/Shell.groovy
> > > ae3da68
> > > bigtop-test-framework/src/test/groovy/org/apache/bigtop/itest/shell/ShellTest.groovy
> > > 1571e10
> > >
> > > Diff: https://reviews.apache.org/r/9166/diff/
> > >
> > >
> > > Testing
> > > -------
> > >
> > > Added unit tests for new functionality. Current unit tests pass
> > >
> > >
> > > Thanks,
> > >
> > > Hari Shreedharan
