Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2807063004:
[devtools] update what's new text for m59 (Closed)
|
Created:
3 years, 8 months ago by kayce (google) Modified:
3 years, 8 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, kozyatinskiy+blink_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[devtools] update what's new text for m59
BUG=710207
Review-Url: https://codereview.chromium.org/2807063004
Cr-Commit-Position: refs/heads/master@{#464249}
Committed: https://chromium.googlesource.com/chromium/src/+/8756d7b0d73dc3571ad29a4d1b22ff8e35eeabb8
Patch Set 1 #
Total comments: 13
Patch Set 2 : update per feedback #
Total comments: 1
Patch Set 3 : update what's new image #Patch Set 4 : imago optimizo magnifico #
Messages
Total messages: 16 (6 generated)
kayce@google.com changed reviewers: + chenwilliam@chromium.org
chenwilliam@chromium.org changed reviewers: + pfeldman@chromium.org
Thanks for submitting this. I've included a lot of Pavel's feedback b/c he swung by my desk :) Comments on the image: * The image seems to have an unexpected border (see the screenshot I uploaded in crbug.com/710207) * Pavel told me the trick to making it look good on Retina display is to have the actual image size be twice the dimensions specified in the CSS (https://cs.chromium.org/chromium/src/third_party/WebKit/Source/devtools/front...) https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js (left): https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:11: version: 1, Let's append, not replace - (e.g. create a new object with the same shape). This will help out with cherry-picking changes (if needed). https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js (right): https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:14: { Let's do sentence capitalization for the title (e.g. "Unified command menu"). https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:15: title: 'CSS and JS Coverage', Rename -> CSS and JS code coverage https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:20: title: 'Workspaces 2.0', Move workspace 2.0 to the bottom of the list https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:35: title: 'Async Debugging', Asked Kozy for a better title and he said "Step over async await" https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:40: title: 'Unified Command Menu', Can you include keyboard shortcut for Unified Command Menu? For Windows and Linux, it's "Ctrl+Shift+P" and Mac it's "⌘+Shift+P". You can use "Host.isMac()". https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:45: title: 'Capture Settings', Let's remove capture settings from the highlights
https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... File third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js (right): https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:14: { On 2017/04/10 23:35:51, chenwilliam wrote: > Let's do sentence capitalization for the title (e.g. "Unified command menu"). OK, but Command Menu is a DevTools feature / UI, so that particular example should be "Unified Command Menu". Just like how we capitalized the "Performance" in "Performance panel" from the last highlight. https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:15: title: 'CSS and JS Coverage', On 2017/04/10 23:35:51, chenwilliam wrote: > Rename -> CSS and JS code coverage Done. https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:20: title: 'Workspaces 2.0', On 2017/04/10 23:35:51, chenwilliam wrote: > Move workspace 2.0 to the bottom of the list Done. https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:35: title: 'Async Debugging', On 2017/04/10 23:35:51, chenwilliam wrote: > Asked Kozy for a better title and he said "Step over async await" Done. https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:40: title: 'Unified Command Menu', On 2017/04/10 23:35:51, chenwilliam wrote: > Can you include keyboard shortcut for Unified Command Menu? For Windows and > Linux, it's "Ctrl+Shift+P" and Mac it's "⌘+Shift+P". You can use "Host.isMac()". Done. https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:45: title: 'Capture Settings', On 2017/04/10 23:35:51, chenwilliam wrote: > Let's remove capture settings from the highlights Done.
On 2017/04/11 17:36:18, kayce (google) wrote: > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > File third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js > (right): > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:14: { > On 2017/04/10 23:35:51, chenwilliam wrote: > > Let's do sentence capitalization for the title (e.g. "Unified command menu"). > > OK, but Command Menu is a DevTools feature / UI, so that particular example > should be "Unified Command Menu". Just like how we capitalized the "Performance" > in "Performance panel" from the last highlight. > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:15: title: > 'CSS and JS Coverage', > On 2017/04/10 23:35:51, chenwilliam wrote: > > Rename -> CSS and JS code coverage > > Done. > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:20: title: > 'Workspaces 2.0', > On 2017/04/10 23:35:51, chenwilliam wrote: > > Move workspace 2.0 to the bottom of the list > > Done. > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:35: title: > 'Async Debugging', > On 2017/04/10 23:35:51, chenwilliam wrote: > > Asked Kozy for a better title and he said "Step over async await" > > Done. > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:40: title: > 'Unified Command Menu', > On 2017/04/10 23:35:51, chenwilliam wrote: > > Can you include keyboard shortcut for Unified Command Menu? For Windows and > > Linux, it's "Ctrl+Shift+P" and Mac it's "⌘+Shift+P". You can use > "Host.isMac()". > > Done. > > https://codereview.chromium.org/2807063004/diff/1/third_party/WebKit/Source/d... > third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:45: title: > 'Capture Settings', > On 2017/04/10 23:35:51, chenwilliam wrote: > > Let's remove capture settings from the highlights > > Done. Re: the box around the image... I used Cmd + Shft + 4 and then Spacebar to capture the entire window. My guess is that there's a quirk using that format. So I changed the img to focus in on the coverage table. If this doesn't work, maybe one of you guys should upload the img that meets your specs.
Thanks. I uploaded a modified version of your image on crbug.com/710207 https://codereview.chromium.org/2807063004/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js (right): https://codereview.chromium.org/2807063004/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/devtools/front_end/help/ReleaseNoteText.js:14: version: 2, Can you append instead of prepending it? It'll make the diff cleaner and should work the same. Thanks!
I think prepending is better! lgtm!
Guetzli works on PNGs? I pushed the updated image. I haven't built this, so can you LMK if it builds and looks OK, Will?
The CQ bit was checked by pfeldman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from pfeldman@chromium.org Link to the patchset: https://codereview.chromium.org/2807063004/#ps60001 (title: "imago optimizo magnifico")
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": 60001, "attempt_start_ts": 1492044263440300,
"parent_rev": "3dbb47353998ea483fdea2ff2bd3584d0a25c0f1", "commit_rev":
"8756d7b0d73dc3571ad29a4d1b22ff8e35eeabb8"}
Message was sent while issue was closed.
Description was changed from ========== [devtools] update what's new text for m59 BUG=710207 ========== to ========== [devtools] update what's new text for m59 BUG=710207 Review-Url: https://codereview.chromium.org/2807063004 Cr-Commit-Position: refs/heads/master@{#464249} Committed: https://chromium.googlesource.com/chromium/src/+/8756d7b0d73dc3571ad29a4d1b22... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/8756d7b0d73dc3571ad29a4d1b22... |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
