> 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.
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 ----------------------------------------------------------- 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 > >
