[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack`` of stacktrace buffe…

[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack`` of stacktrace buffer.

BUG=
TBR=stgao@chromium.org

Review-Url: https://codereview.chromium.org/2593383002
Committed: https://chromium.googlesource.com/infra/infra/+/ab06c17ae4d0ac087afe83752add2de2aa7df148

[Sharu Jiang, 2016-12-22 03:12:43]

Patchset #1 (id:1) has been deleted

[Sharu Jiang, 2016-12-22 03:14:19]

katesonia@chromium.org changed reviewers:
+ stgao@chromium.org, wrengr@chromium.org

[Sharu Jiang, 2016-12-22 03:14:19]

PTAL.

[Sharu Jiang, 2016-12-22 03:14:36]

Description was changed from

==========
[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack``
of stacktrace buffer.

BUG=
==========

to

==========
[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack``
of stacktrace buffer.

BUG=
TBR=stgao@chromium.org
==========

[wrengr, 2016-12-22 22:43:00]

lgtm with nits

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/stacktrace.py (right):

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:313: The class can be converted to
immutable stacktrace.
this paragraph should be formatted to avoid weird line breaks. From vim, can use
!}fmt or similar

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:324: signature (str): The signagure is used
to determine the crash stack.
"signagure" -> "signature"

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:325: filters (list of CallStackBuffer):
List of filters to filter stack buffer.
Those aren't CallStackBuffers...

"filters to filter" is awkward

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:336: self.filters = filters
should have a blank line before this, iirc

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:351: for stack_filter in self.filters:
``if filters: for f in filters:...`` -> ``for f in filters or []:...``

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/test/stacktrace_test.py (right):

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/test/stacktrace_test.py:237: """Tests
``AddFilteredStack`` return None for inf priority stack buffer."""
for all these docstrings "Tests ``foo`` return blah" -> "Tests that ``foo``
returns blah"

[Sharu Jiang, 2016-12-23 00:26:20]

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/stacktrace.py (right):

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:313: The class can be converted to
immutable stacktrace.
On 2016/12/22 22:42:59, wrengr wrote:
> this paragraph should be formatted to avoid weird line breaks. From vim, can
use
> !}fmt or similar

Done.

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:324: signature (str): The signagure is used
to determine the crash stack.
On 2016/12/22 22:42:59, wrengr wrote:
> "signagure" -> "signature"

Done.

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:325: filters (list of CallStackBuffer):
List of filters to filter stack buffer.
On 2016/12/22 22:43:00, wrengr wrote:
> Those aren't CallStackBuffers...
> 
> "filters to filter" is awkward

Oops, corrected.

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:336: self.filters = filters
On 2016/12/22 22:42:59, wrengr wrote:
> should have a blank line before this, iirc

Done.

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/stacktrace.py:351: for stack_filter in self.filters:
On 2016/12/22 22:42:59, wrengr wrote:
> ``if filters: for f in filters:...`` -> ``for f in filters or []:...``

Done.

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
File appengine/findit/crash/test/stacktrace_test.py (right):

https://codereview.chromium.org/2593383002/diff/20001/appengine/findit/crash/...
appengine/findit/crash/test/stacktrace_test.py:237: """Tests
``AddFilteredStack`` return None for inf priority stack buffer."""
On 2016/12/22 22:43:00, wrengr wrote:
> for all these docstrings "Tests ``foo`` return blah" -> "Tests that ``foo``
> returns blah"

Done.

[Sharu Jiang, 2016-12-27 18:25:18]

The CQ bit was checked by katesonia@chromium.org

[Sharu Jiang, 2016-12-27 18:25:18]

The patchset sent to the CQ was uploaded after l-g-t-m from wrengr@chromium.org
Link to the patchset: https://codereview.chromium.org/2593383002/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2016-12-27 18:25:24]

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, 2016-12-27 18:34:51]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1482863118286560,
"parent_rev": "49e6c276ef9428af115e0cbd7e77e9a4d2b0d633", "commit_rev":
"ab06c17ae4d0ac087afe83752add2de2aa7df148"}

[commit-bot: I haz the power, 2016-12-27 18:34:54]

Description was changed from

==========
[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack``
of stacktrace buffer.

BUG=
TBR=stgao@chromium.org
==========

to

==========
[Predator] Move ``FilterStackBuffer`` from stack parser to ``AddFilteredStack``
of stacktrace buffer.

BUG=
TBR=stgao@chromium.org

Review-Url: https://codereview.chromium.org/2593383002
Committed:
https://chromium.googlesource.com/infra/infra/+/ab06c17ae4d0ac087afe83752add2...
==========

[commit-bot: I haz the power, 2016-12-27 18:34:55]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/infra/infra/+/ab06c17ae4d0ac087afe83752add2...