Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2121463002:
Fix minor structural defects in protocol.json files. The structure has been verified by typescript'… (Closed)
|
Created:
4 years, 5 months ago by nojvek Modified:
4 years, 4 months ago 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, sergeyv+blink_chromium.org, kozyatinskiy+blink_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionFix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker.
See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator/protocol.d.ts
BUG=625341
Committed: https://crrev.com/f68e2a5da114922dd006bd3356a43efd5e93677b
Cr-Commit-Position: refs/heads/master@{#403952}
Patch Set 1 #
Total comments: 1
Patch Set 2 : Revert iff description edit #Patch Set 3 : [DevTools] add notifyContextDestroyed to V8Inspector #
Messages
Total messages: 33 (10 generated)
Description was changed from ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 ========== to ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 ==========
nojvek@gmail.com changed reviewers: + ofrobots@google.com, paulirish@chromium.com, pfeldman@chromium.com
Hi Folks, Just making minor edits to protocol.json files to make the structure consistent. It seems the file is manually edited. I just type checked the json files with a typescript interface to ensure consistency. See https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... On another note: There are parts of the protocol that I don't understand. 1. What does hidden property stand for? 2. What is the purpose of handlers array ?
pfeldman@chromium.org changed reviewers: + pfeldman@chromium.org
You might need to add your name into the AUTHORS list - see chromium contributors guide. Only needs to be done once. https://codereview.chromium.org/2121463002/diff/1/third_party/WebKit/Source/p... File third_party/WebKit/Source/platform/v8_inspector/js_protocol.json (left): https://codereview.chromium.org/2121463002/diff/1/third_party/WebKit/Source/p... third_party/WebKit/Source/platform/v8_inspector/js_protocol.json:53: { "name": "overflow", "type": "boolean", "description": "True iff some of the properties or entries of the original object did not fit." }, https://en.m.wikipedia.org/wiki/If_and_only_if
My name's already there. Assuming this is because this isn't my first commit to the repo. On 2016/07/02 05:23:47, pfeldman_slow wrote: > You might need to add your name into the AUTHORS list - see chromium > contributors guide. Only needs to be done once. > > https://codereview.chromium.org/2121463002/diff/1/third_party/WebKit/Source/p... > File third_party/WebKit/Source/platform/v8_inspector/js_protocol.json (left): > > https://codereview.chromium.org/2121463002/diff/1/third_party/WebKit/Source/p... > third_party/WebKit/Source/platform/v8_inspector/js_protocol.json:53: { "name": > "overflow", "type": "boolean", "description": "True iff some of the properties > or entries of the original object did not fit." }, > https://en.m.wikipedia.org/wiki/If_and_only_if
revert the iff part then and you are good to go.
Isn't the iff part a spelling mistake or is that actually intended? On Saturday, July 2, 2016, <pfeldman@chromium.org> wrote: > revert the iff part then and you are good to go. > > https://codereview.chromium.org/2121463002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Isn't the iff part a spelling mistake or is that actually intended? On Saturday, July 2, 2016, <pfeldman@chromium.org> wrote: > revert the iff part then and you are good to go. > > https://codereview.chromium.org/2121463002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2016/07/02 10:53:59, pfeldman_slow wrote: > revert the iff part then and you are good to go. Seems I can't reply from my gmail. The chromium email server doesn't like that. Trying again. Is the 'iff' part a spelling mistake or is that actually intended? Regards.
Okay never mind. So the if and only if comment. Reverting. On 2016/07/02 16:35:55, nojvek wrote: > On 2016/07/02 10:53:59, pfeldman_slow wrote: > > revert the iff part then and you are good to go. > > Seems I can't reply from my gmail. The chromium email server doesn't like that. > Trying again. > > Is the 'iff' part a spelling mistake or is that actually intended? > > Regards.
Done. On 2016/07/02 16:37:02, nojvek wrote: > Okay never mind. So the if and only if comment. > Reverting. > > On 2016/07/02 16:35:55, nojvek wrote: > > On 2016/07/02 10:53:59, pfeldman_slow wrote: > > > revert the iff part then and you are good to go. > > > > Seems I can't reply from my gmail. The chromium email server doesn't like > that. > > Trying again. > > > > Is the 'iff' part a spelling mistake or is that actually intended? > > > > Regards.
The CQ bit was checked by pfeldman@chromium.org
lgtm
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
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
Some of the failing tests seem unrelated. Are they just flakey tests? On Saturday, July 2, 2016, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > Try jobs failed on following builders: > win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, > > http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_... > ) > > https://codereview.chromium.org/2121463002/ > -- You received this message because you are subscribed to the Google Groups "Blink Reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to blink-reviews+unsubscribe@chromium.org.
Some of the failing tests seem unrelated. Are they just flakey tests? On Saturday, July 2, 2016, commit-bot@chromium.org via codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote: > Try jobs failed on following builders: > win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, > > http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_... > ) > > https://codereview.chromium.org/2121463002/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The CQ bit was checked by pfeldman@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
On 2016/07/02 18:00:07, commit-bot: I haz the power wrote: > CQ is trying da patch. Follow status at > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... Hi Pavel, Any chance you can shed some light on these? There are parts of the protocol that I don't understand. 1. What does hidden property stand for? Internal use by chrome only? 2. What is the purpose of handlers array ?
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)
There is something wrong with the bots :( I'll file a bug when I get to my eng box.
The CQ bit was checked by pfeldman@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 ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 ========== to ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 ========== to ========== Fix minor structural defects in protocol.json files. The structure has been verified by typescript's type checker. See: https://github.com/nojvek/chrome-remote-debug-interface/blob/master/generator... BUG=625341 Committed: https://crrev.com/f68e2a5da114922dd006bd3356a43efd5e93677b Cr-Commit-Position: refs/heads/master@{#403952} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/f68e2a5da114922dd006bd3356a43efd5e93677b Cr-Commit-Position: refs/heads/master@{#403952}
Message was sent while issue was closed.
On 2016/07/06 21:06:05, commit-bot: I haz the power wrote: > CQ bit was unchecked. Thanks Pavel. Just before I close the thread. Is it correct that hidden properties and methods in the protocol.json are not for public consumption. They can be removed at any time without warning? Regards. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
