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

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. 11, 2013, 7:18 p.m., Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9174/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2013, 7:18 p.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 
> 
> 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
> 
>

Reply via email to