Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(193)

Issue 416983004: Add arch support for tombstone.py (Closed)

Created:
6 years, 5 months ago by haltonhuo
Modified:
6 years, 4 months ago
Reviewers:
jbudorick
CC:
chromium-reviews, klundberg+watch_chromium.org, yfriedman+watch_chromium.org, ilevy-cc_chromium.org
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Project:
chromium
Visibility:
Public.

Description

Add arch support for tombstone.py 'stack' tool has archtechture support by passing --arch argument, if it is not given, arm will be used by default. Thus the stack trace will be wrongly reported. To resolve it, 'arch' prop is added to tombstone when fetching from device, and pass it to symbol.ARCH at end. R=jbudorick@chromium.org BUG= Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=285910

Patch Set 1 #

Total comments: 3

Patch Set 2 : Rework based on jbudorick's comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -3 lines) Patch
M build/android/tombstones.py View 1 5 chunks +7 lines, -3 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
haltonhuo
6 years, 5 months ago (2014-07-25 03:48:26 UTC) #1
jbudorick
https://codereview.chromium.org/416983004/diff/1/build/android/tombstones.py File build/android/tombstones.py (right): https://codereview.chromium.org/416983004/diff/1/build/android/tombstones.py#newcode56 build/android/tombstones.py:56: def _GetDeviceArch(device): This function isn't needed. https://codereview.chromium.org/416983004/diff/1/build/android/tombstones.py#newcode100 build/android/tombstones.py:100: include_stack: ...
6 years, 5 months ago (2014-07-25 14:19:02 UTC) #2
haltonhuo
jbudorick, many thanks for your review, all your comments are addressed in patch set2, pleas ...
6 years, 4 months ago (2014-07-28 02:03:45 UTC) #3
haltonhuo
jbudorick, many thanks for your review, all your comments are addressed in patch set2, please ...
6 years, 4 months ago (2014-07-28 02:03:47 UTC) #4
jbudorick
lgtm Thanks for the patch!
6 years, 4 months ago (2014-07-28 14:21:04 UTC) #5
haltonhuo
The CQ bit was checked by halton.huo@intel.com
6 years, 4 months ago (2014-07-28 14:23:37 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/halton.huo@intel.com/416983004/20001
6 years, 4 months ago (2014-07-28 14:24:23 UTC) #7
commit-bot: I haz the power
6 years, 4 months ago (2014-07-28 15:35:22 UTC) #8
Message was sent while issue was closed.
Change committed as 285910

Powered by Google App Engine
This is Rietveld 408576698