git-cl: show dirty files if any

git-cl: show dirty files if any

R=chase@chromium.org,szager@chromium.org,maruel@chromium.org
BUG=157398
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=164026

[M-A Ruel, 2012-10-25 01:59:03]

Also see https://chromiumcodereview.appspot.com/11275022/. Please coordinate
whoever wants to win.

I'd prefer to refactor the duplicate code into a function.

[ukai, 2012-10-25 02:14:33]

On 2012/10/25 01:59:03, Marc-Antoine Ruel wrote:
> Also see https://chromiumcodereview.appspot.com/11275022/. Please coordinate
> whoever wants to win.
> 
> I'd prefer to refactor the duplicate code into a function.
Done.

[M-A Ruel, 2012-10-25 02:26:33]

lgtm

http://codereview.chromium.org/11262004/diff/3001/git_cl.py
File git_cl.py (right):

http://codereview.chromium.org/11262004/diff/3001/git_cl.py#newcode153
git_cl.py:153: def is_dirty_git_tree(cmd, force=False):
Remove the force argument.

http://codereview.chromium.org/11262004/diff/3001/git_cl.py#newcode1011
git_cl.py:1011: if is_dirty_git_tree('presubmit', force=options.force):
if not options.force and is_dirty_git_tree('presubmit'):

[ukai, 2012-10-25 02:29:47]

http://codereview.chromium.org/11262004/diff/3001/git_cl.py
File git_cl.py (right):

http://codereview.chromium.org/11262004/diff/3001/git_cl.py#newcode153
git_cl.py:153: def is_dirty_git_tree(cmd, force=False):
On 2012/10/25 02:26:33, Marc-Antoine Ruel wrote:
> Remove the force argument.

Done.

http://codereview.chromium.org/11262004/diff/3001/git_cl.py#newcode1011
git_cl.py:1011: if is_dirty_git_tree('presubmit', force=options.force):
On 2012/10/25 02:26:33, Marc-Antoine Ruel wrote:
> if not options.force and is_dirty_git_tree('presubmit'):

Done.

[Isaac (away), 2012-10-25 06:15:26]

Looks like we were both working on the same CL (mine is
https://chromiumcodereview.appspot.com/11275022/).

I have some suggestions that come from mine.  But looks good!

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py#newcode159
git_cl.py:159: print 'Uncommitted files:'
nit, could we print the command we used here?

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py#newcode160
git_cl.py:160: print dirty
You should clip this in case it is very long.  Maybe:

print dirty[:4096]

[ukai, 2012-10-25 06:22:30]

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py#newcode159
git_cl.py:159: print 'Uncommitted files:'
On 2012/10/25 06:15:26, Isaac wrote:
> nit, could we print the command we used here?

Done.

https://chromiumcodereview.appspot.com/11262004/diff/7001/git_cl.py#newcode160
git_cl.py:160: print dirty
On 2012/10/25 06:15:26, Isaac wrote:
> You should clip this in case it is very long.  Maybe:
> 
> print dirty[:4096]

Done.

[Isaac (away), 2012-10-25 07:30:31]

Thanks!  lgtm

[Isaac (away), 2012-10-25 07:53:20]

http://codereview.chromium.org/11262004/diff/10001/git_cl.py
File git_cl.py (right):

http://codereview.chromium.org/11262004/diff/10001/git_cl.py#newcode160
git_cl.py:160: print dirty[:4096]
actually 4096 is pretty long. Maybe we want shorter if users can copy/paste the
command?  Either way is fine.