Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1403803002:
SkStringPrintf and SkString::printf now are no longer limted by a static buffer (Closed)
|
Created:
5 years, 2 months ago by hal.canary Modified:
4 years, 8 months ago CC:
reviews_skia.org Base URL:
https://skia.googlesource.com/skia.git@master Target Ref:
refs/heads/master Project:
skia Visibility:
Public. |
DescriptionSkStringPrintf and SkString::printf now are no longer limted by a static buffer
GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&issue=1403803002
Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c
Committed: https://skia.googlesource.com/skia/+/d51bdae4e145bfede693f97cf0d54a56d33d3c9e
Patch Set 1 #Patch Set 2 : 2015-10-12 (Monday) 15:52:58 EDT #
Total comments: 2
Patch Set 3 : 2015-10-12 (Monday) 17:37:19 EDT #Patch Set 4 : 2016-04-22 (Friday) 10:31:37 EDT #Patch Set 5 : 2016-04-22 (Friday) 10:48:27 EDT #Patch Set 6 : 2016-04-25 (Monday) 11:33:51 EDT #
Messages
Total messages: 38 (19 generated)
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1403803002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1403803002/1
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: Build-Win-MSVC-x86_64-Debug-Trybot on client.skia.compile (JOB_FAILED, http://build.chromium.org/p/client.skia.compile/builders/Build-Win-MSVC-x86_6...)
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1403803002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1403803002/20001
The CQ bit was unchecked by halcanary@google.com
The CQ bit was checked by halcanary@google.com
The CQ bit was unchecked by halcanary@google.com
halcanary@google.com changed reviewers: + tomhudson@google.com
PTAL
reed@google.com changed reviewers: + reed@google.com
unittest?
https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp File src/core/SkString.cpp (right): https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp#n... src/core/SkString.cpp:659: // Improvement on SkStringPrintf to allow for arbitrarily long output. This feels like a comment that belongs in the commit note, rather than the source file. How can SkStringPrintf be an improvement on SkStringPrintf? Rather, the change you're making is an improvement, right?
On 2015/10/12 20:21:01, tomhudson wrote: > https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp > File src/core/SkString.cpp (right): > > https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp#n... > src/core/SkString.cpp:659: // Improvement on SkStringPrintf to allow for > arbitrarily long output. > This feels like a comment that belongs in the commit note, rather than the > source file. How can SkStringPrintf be an improvement on SkStringPrintf? Rather, > the change you're making is an improvement, right? cut-and-paste error.
On 2015/10/12 20:15:24, reed1 wrote: > unittest? done
https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp File src/core/SkString.cpp (right): https://codereview.chromium.org/1403803002/diff/20001/src/core/SkString.cpp#n... src/core/SkString.cpp:659: // Improvement on SkStringPrintf to allow for arbitrarily long output. On 2015/10/12 20:21:01, tomhudson wrote: > This feels like a comment that belongs in the commit note, rather than the > source file. How can SkStringPrintf be an improvement on SkStringPrintf? Rather, > the change you're making is an improvement, right? Done.
Also, the unit test uncovered a bug, which required me to refactor again.
lgtm
On 2015/10/13 17:49:10, tomhudson wrote: > lgtm This appears to never have landed - is it still relevant? Can we close it?
On 2016/04/22 14:05:22, tomhudson wrote: > On 2015/10/13 17:49:10, tomhudson wrote: > > lgtm > > This appears to never have landed - is it still relevant? Can we close it? It is more relevant now that I finally need it. I'll rebase and land later today.
Description was changed from ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer ========== to ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
Description was changed from ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ==========
The CQ bit was checked by halcanary@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from tomhudson@google.com Link to the patchset: https://codereview.chromium.org/1403803002/#ps80001 (title: "2016-04-22 (Friday) 10:48:27 EDT")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1403803002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1403803002/80001
Message was sent while issue was closed.
Description was changed from ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... ========== to ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c ==========
Message was sent while issue was closed.
Committed patchset #5 (id:80001) as https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c
Message was sent while issue was closed.
A revert of this CL (patchset #5 id:80001) has been created in https://codereview.chromium.org/1908423002/ by halcanary@google.com. The reason for reverting is: breaking something.
Message was sent while issue was closed.
Description was changed from ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c ========== to ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c ==========
The CQ bit was checked by halcanary@google.com to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1403803002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1403803002/100001
The CQ bit was unchecked by halcanary@google.com
The CQ bit was checked by halcanary@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from tomhudson@google.com Link to the patchset: https://codereview.chromium.org/1403803002/#ps100001 (title: "2016-04-25 (Monday) 11:33:51 EDT")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1403803002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1403803002/100001
Message was sent while issue was closed.
Description was changed from ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c ========== to ========== SkStringPrintf and SkString::printf now are no longer limted by a static buffer GOLD_TRYBOT_URL= https://gold.skia.org/search2?unt=true&query=source_type%3Dgm&master=false&is... Committed: https://skia.googlesource.com/skia/+/606cadd5aac62299ef2e277709b3684cae2bf96c Committed: https://skia.googlesource.com/skia/+/d51bdae4e145bfede693f97cf0d54a56d33d3c9e ==========
Message was sent while issue was closed.
Committed patchset #6 (id:100001) as https://skia.googlesource.com/skia/+/d51bdae4e145bfede693f97cf0d54a56d33d3c9e |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
