Refactoring: Split up "commit_changes_if_needed" method.

Refactoring: Split up "commit_changes_if_needed" method.

Purpose: The purpose of this change is to improve readability
and also make it easier to test the commit message, which I
think we want to change for bug 682340.

BUG=682340
Review-Url: https://codereview.chromium.org/2639213003
Cr-Commit-Position: refs/heads/master@{#444810}
Committed: https://chromium.googlesource.com/chromium/src/+/7ea227fdc0908323f9c1b43918368fe04cdc1bf3

[qyearsley, 2017-01-18 19:18:22]

qyearsley@chromium.org changed reviewers:
+ jeffcarp@chromium.org

[jeffcarp, 2017-01-18 19:45:40]

lgtm

https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):

https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:243:
self.run(['git', 'commit', '-a', '-F', path_to_commit_msg])
Not sure if this is preferable, but you can also pipe the commit message to the
stdin of git commit, e.g.

self.run(['git', 'commit', '-a', '-F', '-'], stdin=commit_message)

Docs:
https://git-scm.com/docs/git-commit#git-commit--Fltfilegt

[qyearsley, 2017-01-18 22:25:33]

On 2017/01/18 at 19:45:40, jeffcarp wrote:
> lgtm
> 
>
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):
> 
>
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:243:
self.run(['git', 'commit', '-a', '-F', path_to_commit_msg])
> Not sure if this is preferable, but you can also pipe the commit message to
the stdin of git commit, e.g.
> 
> self.run(['git', 'commit', '-a', '-F', '-'], stdin=commit_message)
> 
> Docs:
> https://git-scm.com/docs/git-commit#git-commit--Fltfilegt

Oh, that does sound preferable :-) I'd like to try that.

[qyearsley, 2017-01-18 23:03:52]

The CQ bit was checked by qyearsley@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-01-18 23:04:58]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[qyearsley, 2017-01-18 23:05:14]

On 2017/01/18 at 22:25:33, qyearsley wrote:
> On 2017/01/18 at 19:45:40, jeffcarp wrote:
> > lgtm
> > 
> >
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> > File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py (right):
> > 
> >
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> > third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:243:
self.run(['git', 'commit', '-a', '-F', path_to_commit_msg])
> > Not sure if this is preferable, but you can also pipe the commit message to
the stdin of git commit, e.g.
> > 
> > self.run(['git', 'commit', '-a', '-F', '-'], stdin=commit_message)
> > 
> > Docs:
> > https://git-scm.com/docs/git-commit#git-commit--Fltfilegt
> 
> Oh, that does sound preferable :-) I'd like to try that.

Now modified to do this -- although I suspect that it's probably not necessary
to get
the commit message like this, and maybe it would be better to add a commit
method to the Git module and use that. WDYT?

[commit-bot: I haz the power, 2017-01-19 00:26:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-01-19 00:26:02]

Dry run: This issue passed the CQ dry run.

[jeffcarp, 2017-01-19 19:03:58]

On 2017/01/18 at 23:05:14, qyearsley wrote:
> On 2017/01/18 at 22:25:33, qyearsley wrote:
> > On 2017/01/18 at 19:45:40, jeffcarp wrote:
> > > lgtm
> > > 
> > >
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> > > File third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py
(right):
> > > 
> > >
https://codereview.chromium.org/2639213003/diff/1/third_party/WebKit/Tools/Sc...
> > > third_party/WebKit/Tools/Scripts/webkitpy/w3c/deps_updater.py:243:
self.run(['git', 'commit', '-a', '-F', path_to_commit_msg])
> > > Not sure if this is preferable, but you can also pipe the commit message
to the stdin of git commit, e.g.
> > > 
> > > self.run(['git', 'commit', '-a', '-F', '-'], stdin=commit_message)
> > > 
> > > Docs:
> > > https://git-scm.com/docs/git-commit#git-commit--Fltfilegt
> > 
> > Oh, that does sound preferable :-) I'd like to try that.
> 
> Now modified to do this -- although I suspect that it's probably not necessary
to get
> the commit message like this, and maybe it would be better to add a commit
method to the Git module and use that. WDYT?

Looks good! Adding a commit method to the Git module sounds good but I don't
think it has to be part of this CL. It could be done under the umbrella of
https://bugs.chromium.org/p/chromium/issues/detail?id=676399

[qyearsley, 2017-01-19 19:05:26]

The CQ bit was checked by qyearsley@chromium.org

[qyearsley, 2017-01-19 19:05:26]

The patchset sent to the CQ was uploaded after l-g-t-m from
jeffcarp@chromium.org
Link to the patchset: https://codereview.chromium.org/2639213003/#ps20001
(title: "Use stdin pipe.")

[commit-bot: I haz the power, 2017-01-19 19:05:52]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-19 19:11:19]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1484852726036920,
"parent_rev": "1de87f4fa5ef5b9d4a6eb7e0846d05448b438a73", "commit_rev":
"7ea227fdc0908323f9c1b43918368fe04cdc1bf3"}

[commit-bot: I haz the power, 2017-01-19 19:11:48]

Description was changed from

==========
Refactoring: Split up "commit_changes_if_needed" method.

Purpose: The purpose of this change is to improve readability
and also make it easier to test the commit message, which I
think we want to change for bug 682340.

BUG=682340
==========

to

==========
Refactoring: Split up "commit_changes_if_needed" method.

Purpose: The purpose of this change is to improve readability
and also make it easier to test the commit message, which I
think we want to change for bug 682340.

BUG=682340

Review-Url: https://codereview.chromium.org/2639213003
Cr-Commit-Position: refs/heads/master@{#444810}
Committed:
https://chromium.googlesource.com/chromium/src/+/7ea227fdc0908323f9c1b4391836...
==========

[commit-bot: I haz the power, 2017-01-19 19:11:50]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/7ea227fdc0908323f9c1b4391836...