Exclude codereview.settings from Crashpad

Exclude codereview.settings from Crashpad

BUG=697185
Review-Url: https://codereview.chromium.org/2722063003
Cr-Commit-Position: refs/heads/master@{#454620}
Committed: https://chromium.googlesource.com/chromium/src/+/6af56c56185b4ca8518353f49c43121bda5f8b0d

[Mark Mentovai, 2017-03-01 16:49:58]

mark@chromium.org changed reviewers:
+ agable@chromium.org

[scottmg, 2017-03-01 18:38:15]

scottmg@chromium.org changed reviewers:
+ scottmg@chromium.org

[scottmg, 2017-03-01 18:38:15]

lgtm

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update.py
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update...
third_party/crashpad/update.py:122: ['git', 'cat-file', 'blob', revision_old +
':' + excluded_file],
I can imagine this mucking up newlines on Windows, but I don't know either way.
I'll try it out next time I roll.

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update...
third_party/crashpad/update.py:127: with open(excluded_full_path, 'wbx') as
file:
I've never used 'x' before, why is it useful here?

[agable, 2017-03-01 20:50:54]

High-level comment: this seems too complex. It seems like you could do the whole
copy just like before, and then before generating the commit do "git checkout
HEAD -- third_party/crashpad/crashpad/codereview.settings" to reset that file to
its state before the copy in.

All of this cat-file/write-file/commit-just-excluded-files stuff seems like
unnecessary work and confusion to anyone reading the script.

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update.py
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update...
third_party/crashpad/update.py:124: excluded_full_path = \
Don't use \ for line continuations, use parentheses instead.

[Mark Mentovai, 2017-03-01 21:00:37]

On 2017/03/01 20:50:54, agable wrote:
> High-level comment: this seems too complex. It seems like you could do the
whole
> copy just like before, and then before generating the commit do "git checkout
> HEAD -- third_party/crashpad/crashpad/codereview.settings" to reset that file
to
> its state before the copy in.

“Like before” wasn’t/isn’t a copy, it’s a commit-by-commit cherry-pick. If one
of the commits in the range touches codereview.settings, then the cherry-pick
will trip over it. That’ll be even harder to unravel.

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update.py
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/1/third_party/crashpad/update...
third_party/crashpad/update.py:127: with open(excluded_full_path, 'wbx') as
file:
scottmg wrote:
> I've never used 'x' before, why is it useful here?

It’s O_EXCL, but it also probably doesn’t work on all platforms.

I’m just using it to assert that the file isn’t already there. I can take it out
or use a more portable approach if you’d prefer.

[agable, 2017-03-01 21:09:38]

On 2017/03/01 at 21:00:37, mark wrote:
> On 2017/03/01 20:50:54, agable wrote:
> > High-level comment: this seems too complex. It seems like you could do the
whole
> > copy just like before, and then before generating the commit do "git
checkout
> > HEAD -- third_party/crashpad/crashpad/codereview.settings" to reset that
file to
> > its state before the copy in.
> 
> “Like before” wasn’t/isn’t a copy, it’s a commit-by-commit cherry-pick. If one
of the commits in the range touches codereview.settings, then the cherry-pick
will trip over it. That’ll be even harder to unravel.

Why? The final result is a single commit saying "Update crashpad to <hash>" like
https://chromium.googlesource.com/chromium/src/+/d0b0e164df66904125ecdcf34732....
There's no reason to do this as a series of git operations instead of a plain
file copy from a checkout of crashpad elsewhere on disk.

But even if you don't want to modify the fundamental operating mode of this
script, all this complexity is still unnecessary. It would be much simpler, and
much clearer to the reader, to run a "git filter-branch" against the crashpad
commits to exclude all changes to codereview.settings before cherry-picking them
over.

[Mark Mentovai, 2017-03-01 21:30:05]

On 2017/03/01 21:09:38, agable wrote:
> On 2017/03/01 at 21:00:37, mark wrote:
> > “Like before” wasn’t/isn’t a copy, it’s a commit-by-commit cherry-pick. If
one
> of the commits in the range touches codereview.settings, then the cherry-pick
> will trip over it. That’ll be even harder to unravel.
> 
> Why? The final result is a single commit saying "Update crashpad to <hash>"
like
>
https://chromium.googlesource.com/chromium/src/+/d0b0e164df66904125ecdcf34732....
> There's no reason to do this as a series of git operations instead of a plain
> file copy from a checkout of crashpad elsewhere on disk.

That wouldn’t allow an import to not clobber local modifications, which was an
important design goal. Otherwise, yes, we would have just obliterated everything
and copied the new version right over, and it would have been much simpler.

> But even if you don't want to modify the fundamental operating mode of this
> script, all this complexity is still unnecessary. It would be much simpler,
and
> much clearer to the reader, to run a "git filter-branch" against the crashpad
> commits to exclude all changes to codereview.settings before cherry-picking
them
> over.

How it is now is how Primiano suggested we set it up when we switched from being
DEPSed to doing this thing we’re doing now. Can what you’re suggesting preserve
local modifications? Can it still detect whether Chrome’s copy of Crashpad has
diverged for README.chromium maintenance and as a signal that maybe there are
local changes in Chrome that need to be brought back to upstream Crashpad?

[agable, 2017-03-01 21:39:00]

On 2017/03/01 at 21:30:05, mark wrote:
> On 2017/03/01 21:09:38, agable wrote:
> > On 2017/03/01 at 21:00:37, mark wrote:
> > > “Like before” wasn’t/isn’t a copy, it’s a commit-by-commit cherry-pick. If
one
> > of the commits in the range touches codereview.settings, then the
cherry-pick
> > will trip over it. That’ll be even harder to unravel.
> > 
> > Why? The final result is a single commit saying "Update crashpad to <hash>"
like
> >
https://chromium.googlesource.com/chromium/src/+/d0b0e164df66904125ecdcf34732....
> > There's no reason to do this as a series of git operations instead of a
plain
> > file copy from a checkout of crashpad elsewhere on disk.
> 
> That wouldn’t allow an import to not clobber local modifications, which was an
important design goal. Otherwise, yes, we would have just obliterated everything
and copied the new version right over, and it would have been much simpler.

That makes sense (although I'll note that the README.chromium explicitly states
that local modifications *will* be clobbered, and that strikes me as a good
thing to force people to upstream them, so going out of the way to preserve them
seems like unnecessary effort).

> 
> > But even if you don't want to modify the fundamental operating mode of this
> > script, all this complexity is still unnecessary. It would be much simpler,
and
> > much clearer to the reader, to run a "git filter-branch" against the
crashpad
> > commits to exclude all changes to codereview.settings before cherry-picking
them
> > over.
> 
> How it is now is how Primiano suggested we set it up when we switched from
being DEPSed to doing this thing we’re doing now. Can what you’re suggesting
preserve local modifications? Can it still detect whether Chrome’s copy of
Crashpad has diverged for README.chromium maintenance and as a signal that maybe
there are local changes in Chrome that need to be brought back to upstream
Crashpad?

Yes, it would be exactly the same process as the script does today, but with a
single extra step that removes modifications to codereview.settings from all the
crashpad commits that are going to be cherry-picked. Modifications to other
files would still be treated exactly the same as today.

[Mark Mentovai, 2017-03-01 21:42:41]

On 2017/03/01 21:39:00, agable wrote:
> On 2017/03/01 at 21:30:05, mark wrote:
> > That wouldn’t allow an import to not clobber local modifications, which was
an
> important design goal. Otherwise, yes, we would have just obliterated
everything
> and copied the new version right over, and it would have been much simpler.
> 
> That makes sense (although I'll note that the README.chromium explicitly
states
> that local modifications *will* be clobbered, and that strikes me as a good
> thing to force people to upstream them, so going out of the way to preserve
them
> seems like unnecessary effort).

This is intentional. We want to scare people into doing the right thing, putting
the burden on them to upstream their changes. But if they don’t, we want to take
the burden off of ourselves for breaking something when we run the updater.
Little white lie.

> > How it is now is how Primiano suggested we set it up when we switched from
> being DEPSed to doing this thing we’re doing now. Can what you’re suggesting
> preserve local modifications? Can it still detect whether Chrome’s copy of
> Crashpad has diverged for README.chromium maintenance and as a signal that
maybe
> there are local changes in Chrome that need to be brought back to upstream
> Crashpad?
> 
> Yes, it would be exactly the same process as the script does today, but with a
> single extra step that removes modifications to codereview.settings from all
the
> crashpad commits that are going to be cherry-picked. Modifications to other
> files would still be treated exactly the same as today.

OK. I have limited experience with git-filter-branch. Would you be cool if we
landed this as-is and then we can circle back to simplify the script in that
way? Or is what you’re suggesting literally so easy that we can do it instead of
this change?

[agable, 2017-03-01 22:03:22]

On 2017/03/01 at 21:42:41, mark wrote:
> On 2017/03/01 21:39:00, agable wrote:
> > On 2017/03/01 at 21:30:05, mark wrote:
> > > That wouldn’t allow an import to not clobber local modifications, which
was an
> > important design goal. Otherwise, yes, we would have just obliterated
everything
> > and copied the new version right over, and it would have been much simpler.
> > 
> > That makes sense (although I'll note that the README.chromium explicitly
states
> > that local modifications *will* be clobbered, and that strikes me as a good
> > thing to force people to upstream them, so going out of the way to preserve
them
> > seems like unnecessary effort).
> 
> This is intentional. We want to scare people into doing the right thing,
putting the burden on them to upstream their changes. But if they don’t, we want
to take the burden off of ourselves for breaking something when we run the
updater. Little white lie.
> 
> > > How it is now is how Primiano suggested we set it up when we switched from
> > being DEPSed to doing this thing we’re doing now. Can what you’re suggesting
> > preserve local modifications? Can it still detect whether Chrome’s copy of
> > Crashpad has diverged for README.chromium maintenance and as a signal that
maybe
> > there are local changes in Chrome that need to be brought back to upstream
> > Crashpad?
> > 
> > Yes, it would be exactly the same process as the script does today, but with
a
> > single extra step that removes modifications to codereview.settings from all
the
> > crashpad commits that are going to be cherry-picked. Modifications to other
> > files would still be treated exactly the same as today.
> 
> OK. I have limited experience with git-filter-branch. Would you be cool if we
landed this as-is and then we can circle back to simplify the script in that
way? Or is what you’re suggesting literally so easy that we can do it instead of
this change?

Yeah, it should just be subprocess.check_call(['git', 'filter-branch',
'--index-filter', 'git rm --cached --ignore-unmatch codereview.settings',
'%s..%s' % (revision_old, parsed.update_to)]) That basically says "for every
commit from revision_old to parsed.update_to, create a new commit that looks the
same except right before committing it run `git rm codereview.settings`".

[Mark Mentovai, 2017-03-01 22:14:34]

On 2017/03/01 22:03:22, agable wrote:
> On 2017/03/01 at 21:42:41, mark wrote:
> > OK. I have limited experience with git-filter-branch. Would you be cool if
we
> landed this as-is and then we can circle back to simplify the script in that
> way? Or is what you’re suggesting literally so easy that we can do it instead
of
> this change?
> 
> Yeah, it should just be subprocess.check_call(['git', 'filter-branch',
> '--index-filter', 'git rm --cached --ignore-unmatch codereview.settings',
> '%s..%s' % (revision_old, parsed.update_to)]) That basically says "for every
> commit from revision_old to parsed.update_to, create a new commit that looks
the
> same except right before committing it run `git rm codereview.settings`".

And how do I test for local modifications, what I’m currently using git
diff-tree for? If I don’t have a copy of codereview.settings matching what’s at
crashpad’s master, diff-tree will say that there’s a difference.

[agable, 2017-03-01 22:29:57]

On 2017/03/01 at 22:14:34, mark wrote:
> On 2017/03/01 22:03:22, agable wrote:
> > On 2017/03/01 at 21:42:41, mark wrote:
> > > OK. I have limited experience with git-filter-branch. Would you be cool if
we
> > landed this as-is and then we can circle back to simplify the script in that
> > way? Or is what you’re suggesting literally so easy that we can do it
instead of
> > this change?
> > 
> > Yeah, it should just be subprocess.check_call(['git', 'filter-branch',
> > '--index-filter', 'git rm --cached --ignore-unmatch codereview.settings',
> > '%s..%s' % (revision_old, parsed.update_to)]) That basically says "for every
> > commit from revision_old to parsed.update_to, create a new commit that looks
the
> > same except right before committing it run `git rm codereview.settings`".
> 
> And how do I test for local modifications, what I’m currently using git
diff-tree for? If I don’t have a copy of codereview.settings matching what’s at
crashpad’s master, diff-tree will say that there’s a difference.

My point is that there shouldn't be a codereview.settings file at all for this
directory of chromium/src. If changes have been made to it -- either on the
crashpad *or* on the chromium side (which would be impossible if it doesn't
exist) -- it shouldn't matter.

[Mark Mentovai, 2017-03-01 22:34:00]

On 2017/03/01 22:29:57, agable wrote:
> On 2017/03/01 at 22:14:34, mark wrote:
> > And how do I test for local modifications, what I’m currently using git
> diff-tree for? If I don’t have a copy of codereview.settings matching what’s
at
> crashpad’s master, diff-tree will say that there’s a difference.
> 
> My point is that there shouldn't be a codereview.settings file at all for this
> directory of chromium/src. If changes have been made to it -- either on the
> crashpad *or* on the chromium side (which would be impossible if it doesn't
> exist) -- it shouldn't matter.

Right. But how do I detect whether there are any local changes to Chromium’s
copy of Crashpad relative to the upstream copy of Crashpad which DOES have a
codereview.settings? I use git diff-tree to do that. I use the result to clear
the “Local Modifications” section in README.chromium or tell the operator that
there are local modifications that may need to be upstreamed (or at least noted
in README.chromium). In this change, I use git diff-tree immediately before
removing the temporary codereview.settings, which works. If I switch to doing
everything with git filter-branch as you propose, there’s never any
codereview.settings in Chromium’s copy, which is fine, but git diff-tree will
always see that as a difference between the in-Chromium copy and the upstream
copy.

[agable, 2017-03-01 23:24:06]

On 2017/03/01 at 22:34:00, mark wrote:
> On 2017/03/01 22:29:57, agable wrote:
> > On 2017/03/01 at 22:14:34, mark wrote:
> > > And how do I test for local modifications, what I’m currently using git
> > diff-tree for? If I don’t have a copy of codereview.settings matching what’s
at
> > crashpad’s master, diff-tree will say that there’s a difference.
> > 
> > My point is that there shouldn't be a codereview.settings file at all for
this
> > directory of chromium/src. If changes have been made to it -- either on the
> > crashpad *or* on the chromium side (which would be impossible if it doesn't
> > exist) -- it shouldn't matter.
> 
> Right. But how do I detect whether there are any local changes to Chromium’s
copy of Crashpad relative to the upstream copy of Crashpad which DOES have a
codereview.settings? I use git diff-tree to do that. I use the result to clear
the “Local Modifications” section in README.chromium or tell the operator that
there are local modifications that may need to be upstreamed (or at least noted
in README.chromium). In this change, I use git diff-tree immediately before
removing the temporary codereview.settings, which works. If I switch to doing
everything with git filter-branch as you propose, there’s never any
codereview.settings in Chromium’s copy, which is fine, but git diff-tree will
always see that as a difference between the in-Chromium copy and the upstream
copy.

a) Always noting that the file is gone doesn't seem like a bad thing. It's true:
the file is gone. If someone tries to do an import with something other than
this script, they'll know to do the same removal.
b) Do the git diff-tree against the HEAD of the filter-branch'ed version of
crashpad. That one won't have a codereview.settings in it, so it wouldn't be
counted as a diff.

[Mark Mentovai, 2017-03-02 00:05:57]

On 2017/03/01 23:24:06, agable wrote:
> On 2017/03/01 at 22:34:00, mark wrote:
> > Right. But how do I detect whether there are any local changes to Chromium’s
> copy of Crashpad relative to the upstream copy of Crashpad which DOES have a
> codereview.settings? I use git diff-tree to do that. I use the result to clear
> the “Local Modifications” section in README.chromium or tell the operator that
> there are local modifications that may need to be upstreamed (or at least
noted
> in README.chromium). In this change, I use git diff-tree immediately before
> removing the temporary codereview.settings, which works. If I switch to doing
> everything with git filter-branch as you propose, there’s never any
> codereview.settings in Chromium’s copy, which is fine, but git diff-tree will
> always see that as a difference between the in-Chromium copy and the upstream
> copy.
> 
> a) Always noting that the file is gone doesn't seem like a bad thing. It's
true:
> the file is gone. If someone tries to do an import with something other than
> this script, they'll know to do the same removal.

Well, yes, we note that the file is gone (see the currently uploaded patch set).

> b) Do the git diff-tree against the HEAD of the filter-branch'ed version of
> crashpad. That one won't have a codereview.settings in it, so it wouldn't be
> counted as a diff.

OK. One last (I think?) question. If there’s a conflict when cherry-picking, the
script as it stands now stops and asks for help (around line 155 on the “new”
side). This is a very friendly way to deal with the conflict. Is there an
opportunity to ask for help with the filter-branch proposal, or are we basically
stuck throwing the whole lot out and asking the user to replicate all of the
commands manually? (That’d be pretty unfriendly.)

[agable, 2017-03-02 00:13:59]

On 2017/03/02 at 00:05:57, mark wrote:
> On 2017/03/01 23:24:06, agable wrote:
> > On 2017/03/01 at 22:34:00, mark wrote:
> > > Right. But how do I detect whether there are any local changes to
Chromium’s
> > copy of Crashpad relative to the upstream copy of Crashpad which DOES have a
> > codereview.settings? I use git diff-tree to do that. I use the result to
clear
> > the “Local Modifications” section in README.chromium or tell the operator
that
> > there are local modifications that may need to be upstreamed (or at least
noted
> > in README.chromium). In this change, I use git diff-tree immediately before
> > removing the temporary codereview.settings, which works. If I switch to
doing
> > everything with git filter-branch as you propose, there’s never any
> > codereview.settings in Chromium’s copy, which is fine, but git diff-tree
will
> > always see that as a difference between the in-Chromium copy and the
upstream
> > copy.
> > 
> > a) Always noting that the file is gone doesn't seem like a bad thing. It's
true:
> > the file is gone. If someone tries to do an import with something other than
> > this script, they'll know to do the same removal.
> 
> Well, yes, we note that the file is gone (see the currently uploaded patch
set).
> 
> > b) Do the git diff-tree against the HEAD of the filter-branch'ed version of
> > crashpad. That one won't have a codereview.settings in it, so it wouldn't be
> > counted as a diff.
> 
> OK. One last (I think?) question. If there’s a conflict when cherry-picking,
the script as it stands now stops and asks for help (around line 155 on the
“new” side). This is a very friendly way to deal with the conflict. Is there an
opportunity to ask for help with the filter-branch proposal, or are we basically
stuck throwing the whole lot out and asking the user to replicate all of the
commands manually? (That’d be pretty unfriendly.)

It should work the same as how it does today. The process is this:
1) fetch the clusterfuzz commits (say foo..bar)
2) filter-branch them (creating fup..bup)
3) cherry-pick fup..bup instead of foo..bar like you used to
4) if the cherry-picks fail, the user will get the same prompt and will be able
to do the same "cherry-pick --continue" as before

[Mark Mentovai, 2017-03-02 01:23:18]

On 2017/03/02 00:13:59, agable wrote:
> On 2017/03/02 at 00:05:57, mark wrote:
> > OK. One last (I think?) question. If there’s a conflict when cherry-picking,
> the script as it stands now stops and asks for help (around line 155 on the
> “new” side). This is a very friendly way to deal with the conflict. Is there
an
> opportunity to ask for help with the filter-branch proposal, or are we
basically
> stuck throwing the whole lot out and asking the user to replicate all of the
> commands manually? (That’d be pretty unfriendly.)
> 
> It should work the same as how it does today. The process is this:
> 1) fetch the clusterfuzz commits (say foo..bar)
> 2) filter-branch them (creating fup..bup)
> 3) cherry-pick fup..bup instead of foo..bar like you used to
> 4) if the cherry-picks fail, the user will get the same prompt and will be
able
> to do the same "cherry-pick --continue" as before

I got it. I thought that we were trying to replace the cherry-pick part of the
process too, so there was some kind of disconnect between what you were
suggesting and what I thought you were suggesting. Now I can see how this will
work, thanks. I’ll try to simplify the script to use that logic.

Scott and Siggi, sorry for the noise.

[Mark Mentovai, 2017-03-02 02:38:06]

All right, Aaron, I implemented your git-filter-branch suggestion.

I’m not really sure that it’s better or worse than bracketing the cherry-picks
with add/remove. I suppose it’s more gittish, at least. The net ± of patch set 1
was 34, and the net ± of patch set 2 is…drumroll…also 34.

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
third_party/crashpad/update.py:133: 'git rm --cached --ignore-unmatch ' + '
'.join(parsed.exclude),
This line bothers me. How does git-filter-branch interpret it? Does it feed it
to the shell? Is there some quoting that should be applied?

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
third_party/crashpad/update.py:136: shell=IS_WINDOWS)
I’m concerned that the approach might not work on Windows at all, either because
of this line or…just because. I don’t remember why we always use shell=True on
Windows. Maybe Scott does.

[Mark Mentovai, 2017-03-02 02:40:46]

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
third_party/crashpad/update.py:136: shell=IS_WINDOWS)
I wrote:
> I’m concerned that the approach might not work on Windows at all, either
because
> of this line or…just because. I don’t remember why we always use shell=True on
> Windows. Maybe Scott does.

History remembers why: https://codereview.chromium.org/1561213002.

[scottmg, 2017-03-02 03:13:24]

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
File third_party/crashpad/update.py (right):

https://codereview.chromium.org/2722063003/diff/20001/third_party/crashpad/up...
third_party/crashpad/update.py:136: shell=IS_WINDOWS)
On 2017/03/02 02:38:05, Mark Mentovai wrote:
> I’m concerned that the approach might not work on Windows at all, either
because
> of this line or…just because. I don’t remember why we always use shell=True on
> Windows. Maybe Scott does.

Yeah, it's because depot_tools wraps git.exe in git.bat, which won't be found
unless the shell's used to find it.

I'm not sure how that command is run, but I think other git-commands that
themselves run git are run inside a git-bash, so it might be fine on Windows.

[agable, 2017-03-02 18:55:43]

LGTM! Sorry for dragging you through such a drawn-out review, but I really
believe this version is much more readable (and more gitish) than the first
version. And thanks for including awesome comments for the next person who comes
along. Thanks!

[Mark Mentovai, 2017-03-03 01:05:12]

On 2017/03/02 18:55:43, agable wrote:
> LGTM! Sorry for dragging you through such a drawn-out review, but I really
> believe this version is much more readable (and more gitish) than the first
> version. And thanks for including awesome comments for the next person who
comes
> along. Thanks!

Thanks, Aaron. Do you know anything about the question I asked about quoting for
git-filter-branch --index-filter?

[Mark Mentovai, 2017-03-03 17:58:19]

The CQ bit was checked by mark@chromium.org

[Mark Mentovai, 2017-03-03 17:58:19]

The patchset sent to the CQ was uploaded after l-g-t-m from
scottmg@chromium.org, agable@chromium.org
Link to the patchset: https://codereview.chromium.org/2722063003/#ps60001
(title: "Spelling and clarity")

[commit-bot: I haz the power, 2017-03-03 17:58:36]

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-03-03 18:08:19]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1488563899582730,
"parent_rev": "200362b9d0f8bb3748364f8d4938ec6333cf47ab", "commit_rev":
"6af56c56185b4ca8518353f49c43121bda5f8b0d"}

[commit-bot: I haz the power, 2017-03-03 18:09:04]

Description was changed from

==========
Exclude codereview.settings from Crashpad

BUG=697185
==========

to

==========
Exclude codereview.settings from Crashpad

BUG=697185

Review-Url: https://codereview.chromium.org/2722063003
Cr-Commit-Position: refs/heads/master@{#454620}
Committed:
https://chromium.googlesource.com/chromium/src/+/6af56c56185b4ca8518353f49c43...
==========

[commit-bot: I haz the power, 2017-03-03 18:09:05]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/6af56c56185b4ca8518353f49c43...