Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2814383003:
Format ReadMe.txt for ShowThreadNames tool (Closed)
|
Created:
3 years, 8 months ago by chengx Modified:
3 years, 8 months ago Reviewers:
brucedawson CC:
chromium-reviews Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
DescriptionFormat ReadMe.txt for ShowThreadNames tool
BUG=684203
Review-Url: https://codereview.chromium.org/2814383003
Cr-Commit-Position: refs/heads/master@{#464781}
Committed: https://chromium.googlesource.com/chromium/src/+/10df6d1026423b24dab397ff68a80e0ea97f6cfe
Patch Set 1 #
Total comments: 4
Patch Set 2 : Update readme.txt #Messages
Total messages: 15 (9 generated)
Description was changed from ========== Format ReadMe.txt BUG=684203 ========== to ========== Format ReadMe.txt for ShowThreadNames tool BUG=684203 ==========
chengx@chromium.org changed reviewers: + brucedawson@chromium.org
I found a convenient way to help format txt files using notepad++. So I updated this ReadMe.txt a bit. PTAL~
lgtm with some corrections. Aside: I used this tool earlier this week. It was initially failing due to the function not being available even though I was running Anniversary Edition which is supposed to support the function. I then upgraded to Creators Edition and it then worked. And Microsoft needs to start naming all of their threads. https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... File tools/win/ShowThreadNames/ReadMe.txt (right): https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... tools/win/ShowThreadNames/ReadMe.txt:14: SetThreadDescription API call like in Chrome, all thread ID-name pairs are This actually wrong, I think. You should have either: thread ID/name pairs or: thread-ID/name pairs The hyphen joins things, the slash separates them and contrasts them. We want the latter. https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... tools/win/ShowThreadNames/ReadMe.txt:26: users. Once launched, it will show "Please enter the process Id, or "quit" to "the user" reads better than "the users"
On 2017/04/14 18:05:59, brucedawson wrote: > lgtm with some corrections. > > Aside: I used this tool earlier this week. It was initially failing due to the > function not being available even though I was running Anniversary Edition which > is supposed to support the function. I then upgraded to Creators Edition and it > then worked. > > And Microsoft needs to start naming all of their threads. It reminds me that I actually have never run this tool in Anniversary Edition. My desktop was not updated to Anniversary Edition yet, and your laptop was Creators Edition already when I tested this tool. https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... File tools/win/ShowThreadNames/ReadMe.txt (right): https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... tools/win/ShowThreadNames/ReadMe.txt:14: SetThreadDescription API call like in Chrome, all thread ID-name pairs are On 2017/04/14 18:05:59, brucedawson wrote: > This actually wrong, I think. You should have either: > > thread ID/name pairs > > or: > > thread-ID/name pairs > > The hyphen joins things, the slash separates them and contrasts them. We want > the latter. Done. Thanks! https://codereview.chromium.org/2814383003/diff/1/tools/win/ShowThreadNames/R... tools/win/ShowThreadNames/ReadMe.txt:26: users. Once launched, it will show "Please enter the process Id, or "quit" to On 2017/04/14 18:05:59, brucedawson wrote: > "the user" reads better than "the users" Done.
The CQ bit was checked by chengx@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...
lgtm It was weird that the function did not seem to be present on Anniversary Edition. I updated my blog post to say that it became available in Creators Edition, MSDN be damned
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
The CQ bit was checked by chengx@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1492199407701010,
"parent_rev": "de3f80395224d1164851dd3eaaca14b2de8d935c", "commit_rev":
"10df6d1026423b24dab397ff68a80e0ea97f6cfe"}
Message was sent while issue was closed.
Description was changed from ========== Format ReadMe.txt for ShowThreadNames tool BUG=684203 ========== to ========== Format ReadMe.txt for ShowThreadNames tool BUG=684203 Review-Url: https://codereview.chromium.org/2814383003 Cr-Commit-Position: refs/heads/master@{#464781} Committed: https://chromium.googlesource.com/chromium/src/+/10df6d1026423b24dab397ff68a8... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/10df6d1026423b24dab397ff68a8... |
