> On Feb. 11, 2013, 10:08 p.m., Benjamin Hindman wrote: > > hadoop/TUTORIAL.sh, line 119 > > <https://reviews.apache.org/r/9174/diff/6/?file=257710#file257710line119> > > > > I love the run function, but you've changed the semantics of the > > tutorial. We used to show people what we were about to execute and let them > > decide to continue or not. Now we ask them if we should continue, then show > > them what we're going to execute, and run it. Did you know you were > > changing these semantics? What's your reasoning here? > > Vinod Kone wrote: > If you read earlier reviews from BenM, he felt (and I agreed) that > building up summary at the end became too cumbersome. For every change up in > the actual code flow, we had to remember to make the corresponding change in > summary. Having the run() abstractions, lets us correctly build the summary > w/o extra work. > > But, I agree with your concern about the changed semantics. I haven't > thought about it when I did this refactor. > > Two solutions: > > 1) Add a 'read -p' inside run() after the echo. The only problem is that > users have to press enter for every single command! > 2) Keep it as is. I think our descriptions already give users a good > indication of whats happening. > > Vinod Kone wrote: > If you read earlier reviews from BenM, he felt (and I agreed) that > building up summary at the end became too cumbersome. For every change up in > the actual code flow, we had to remember to make the corresponding change in > summary. Having the run() abstractions, lets us correctly build the summary > w/o extra work. > > But, I agree with your concern about the changed semantics. I haven't > thought about it when I did this refactor. > > Two solutions: > > 1) Add a 'read -p' inside run() after the echo. The only problem is that > users have to press enter for every single command! > 2) Keep it as is. I think our descriptions already give users a good > indication of whats happening.
I agree with benh that we want to keep the same semantics. (1) is a good solution. If you need to run commands without the prompt as well, then either: (a) create a prompted_run() that has the prompt, and use both run() / prompted_run() as appropriate (b) add a flag to run for whether to prompt - Ben ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9174/#review16445 ----------------------------------------------------------- On Feb. 14, 2013, 1:19 a.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/9174/ > ----------------------------------------------------------- > > (Updated Feb. 14, 2013, 1:19 a.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > See Summary. > > Also, we only run 'ant' once instead of twice. > > > Diffs > ----- > > hadoop/Makefile.am d1aa75535ab617f9e4a0b8a0db84d77f1916acc4 > hadoop/TUTORIAL.sh 5670d6afa96f858d437f26885e862712bbf72b71 > hadoop/hadoop-2.0.0-mr1-cdh4.1.2_hadoop-env.sh.patch PRE-CREATION > hadoop/hadoop-2.0.0-mr1-cdh4.1.2_mesos.patch PRE-CREATION > src/slave/slave.cpp d4721c3eb51db87278d05f6fbe2eadb8a3a9b4dd > src/tests/fault_tolerance_tests.cpp > 44eef03aedd89ddfce8491add73918d8f453a0f2 > > Diff: https://reviews.apache.org/r/9174/diff/ > > > Testing > ------- > > make hadoop-2.0.0-mr1-cdh4.1.2 > make hadoop-0.20.2-cdh3u3 > make hadoop-0.20.205.0 > > > Thanks, > > Vinod Kone > >
