Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2242233002:
[DevTools] Fix V8 inspector export (Closed)
|
Created:
4 years, 4 months ago by eostroukhov Modified:
4 years, 4 months ago Reviewers:
dgozman CC:
chromium-reviews, caseq+blink_chromium.org, lushnikov+blink_chromium.org, pfeldman+blink_chromium.org, apavlov+blink_chromium.org, devtools-reviews_chromium.org, blink-reviews, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
Description[DevTools] Fix V8 inspector export
R=dgozman@chromium.org
BUG=
Committed: https://crrev.com/ef1ce7140f1d717f3f9d874c679eadf9873a4ccd
Cr-Commit-Position: refs/heads/master@{#412116}
Patch Set 1 #
Total comments: 4
Patch Set 2 : Moved the files to the correct location #Patch Set 3 : Fixed compilation #Patch Set 4 : Now everything builds #
Total comments: 1
Patch Set 5 : Another version #
Messages
Total messages: 34 (19 generated)
https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/inspector_protocol/String16STL.h (right): https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/inspector_protocol/String16STL.h:9: #include <climits> UINT_MAX https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp (left): https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp:60: '<(DEPTH)/third_party/markupsafe/__init__.py', # jinja2 dep They are at a different location in Node. Since they do not change often, there seems to be no reason to add them here.
The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
These files are in v8_inspector, not inspector_protocol. https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp (left): https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp:148: }, Wow! And it all worked...
Moved the files to the correct location
Codereview tool was too smart - it shown me the proper diff even though I effectively moved the files... Please take another look. https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp (left): https://codereview.chromium.org/2242233002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/inspector_protocol/v8_inspector.gyp:148: }, On 2016/08/15 18:44:59, dgozman wrote: > Wow! And it all worked... No it didn't ;)
The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_android on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/cast_shell_a...) linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Fixed compilation
Description was changed from ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= ========== to ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= ==========
eostroukhov@chromium.org changed reviewers: - dgozman@chromium.org
Now everything builds
The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
eostroukhov@chromium.org changed reviewers: + dgozman@chromium.org
Please take a look
Please take a look
lgtm https://codereview.chromium.org/2242233002/diff/60001/third_party/WebKit/Sour... File third_party/WebKit/Source/platform/v8_inspector/v8_inspector.gyp (right): https://codereview.chromium.org/2242233002/diff/60001/third_party/WebKit/Sour... third_party/WebKit/Source/platform/v8_inspector/v8_inspector.gyp:202: '--config', '../v8_inspector/inspector_protocol_config_stl.json', Just "inspector_protocol_config_stl.json".
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Another version
The CQ bit was checked by eostroukhov@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Please take another look. I added operation< to String16 (required so the String16 class could be used as std::map key with libc++) Made the protocol_sources target conditional, as per our offline discussions.
lgtm
The CQ bit was unchecked by eostroukhov@chromium.org
The CQ bit was checked by eostroukhov@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= ========== to ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= ========== to ========== [DevTools] Fix V8 inspector export R=dgozman@chromium.org BUG= Committed: https://crrev.com/ef1ce7140f1d717f3f9d874c679eadf9873a4ccd Cr-Commit-Position: refs/heads/master@{#412116} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/ef1ce7140f1d717f3f9d874c679eadf9873a4ccd Cr-Commit-Position: refs/heads/master@{#412116} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
