Bug 95797 - Remove WTF::String::operator+=
Summary: Remove WTF::String::operator+=
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-09-04 16:52 PDT by Adam Barth
Modified: 2012-09-06 01:24 PDT (History)
8 users (show)

See Also:


Attachments
Patch (1.49 KB, patch)
2012-09-04 16:57 PDT, Adam Barth
no flags Details | Formatted Diff | Diff
Patch (2.02 KB, patch)
2012-09-05 10:50 PDT, Adam Barth
buildbot: commit-queue-
Details | Formatted Diff | Diff
re-spin (2.02 KB, patch)
2012-09-05 15:00 PDT, Adam Barth
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2012-09-04 16:52:18 PDT
Remove WTF::String::operator+=
Comment 1 Adam Barth 2012-09-04 16:57:39 PDT
Created attachment 162133 [details]
Patch
Comment 2 Eric Seidel (no email) 2012-09-04 17:08:40 PDT
Comment on attachment 162133 [details]
Patch

This looks great, once the other stuff lands.  r- to make the EWS not freak out.
Comment 3 Adam Barth 2012-09-05 10:50:17 PDT
Created attachment 162284 [details]
Patch
Comment 4 WebKit Review Bot 2012-09-05 10:53:33 PDT
Attachment 162284 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Pl..." exit_code: 1
Source/WTF/ChangeLog:13:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 5 Eric Seidel (no email) 2012-09-05 10:56:39 PDT
Comment on attachment 162284 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=162284&action=review

> Source/WTF/wtf/Platform.h:1188
> +#if PLATFORM(QT) || PLATFORM(GTK) || PLATFORM(WIN)

You might want to add a comment above this line which explains what this does and that you want to get off this list. :)
Comment 6 Adam Barth 2012-09-05 10:57:03 PDT
(In reply to comment #5)
> (From update of attachment 162284 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=162284&action=review
> 
> > Source/WTF/wtf/Platform.h:1188
> > +#if PLATFORM(QT) || PLATFORM(GTK) || PLATFORM(WIN)
> 
> You might want to add a comment above this line which explains what this does and that you want to get off this list. :)

Will do.
Comment 7 Benjamin Poulain 2012-09-05 11:02:09 PDT
Looks like I mixed my branches yesterday. I'll make one more patch for Mac. I do that ASAP.
Comment 8 Adam Barth 2012-09-05 11:10:04 PDT
(In reply to comment #7)
> Looks like I mixed my branches yesterday. I'll make one more patch for Mac. I do that ASAP.

Thanks!
Comment 9 Build Bot 2012-09-05 11:40:34 PDT
Comment on attachment 162284 [details]
Patch

Attachment 162284 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13765295
Comment 10 Benjamin Poulain 2012-09-05 12:02:09 PDT
(In reply to comment #9)
> (From update of attachment 162284 [details])
> Attachment 162284 [details] did not pass mac-ews (mac):
> Output: http://queues.webkit.org/results/13765295

My bad, I messed up yesterday. Here is the followup fix: https://bugs.webkit.org/show_bug.cgi?id=95880
Comment 11 Gyuyoung Kim 2012-09-05 12:11:48 PDT
Comment on attachment 162284 [details]
Patch

Attachment 162284 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13765304
Comment 12 Adam Barth 2012-09-05 13:42:25 PDT
@kangil: Thoughts on the EFL EWS failure?
Comment 13 Lauro Moura Maranhao Neto 2012-09-05 14:26:48 PDT
(In reply to comment #12)
> @kangil: Thoughts on the EFL EWS failure?

Failed on WebInspectorProxy, alread covered by Benjamin in https://bugs.webkit.org/show_bug.cgi?id=95880
Comment 14 Adam Barth 2012-09-05 14:59:44 PDT
Ok.  Sounds like I should re-upload this patch and try again.
Comment 15 Adam Barth 2012-09-05 15:00:22 PDT
Created attachment 162344 [details]
re-spin
Comment 16 WebKit Review Bot 2012-09-05 15:03:19 PDT
Attachment 162344 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/Pl..." exit_code: 1
Source/WTF/ChangeLog:13:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 17 Adam Barth 2012-09-06 00:13:16 PDT
Thanks for the review!
Comment 18 Adam Barth 2012-09-06 00:20:29 PDT
Comment on attachment 162344 [details]
re-spin

Clearing flags on attachment: 162344

Committed r127702: <http://trac.webkit.org/changeset/127702>
Comment 19 Adam Barth 2012-09-06 00:20:32 PDT
All reviewed patches have been landed.  Closing bug.
Comment 20 mitz 2012-09-06 00:37:50 PDT
(In reply to comment #18)
> (From update of attachment 162344 [details])
> Clearing flags on attachment: 162344
> 
> Committed r127702: <http://trac.webkit.org/changeset/127702>

This broke the debug builds, because Element::formatForDebugger() uses operator+=.
Comment 21 Adam Barth 2012-09-06 00:41:09 PDT
> This broke the debug builds, because Element::formatForDebugger() uses operator+=.

Hum...  I thought paroga had the debug builds fixed.  Maybe his patch didn't land?  /me checks.
Comment 22 Patrick R. Gansterer 2012-09-06 00:41:52 PDT
(In reply to comment #21)
> > This broke the debug builds, because Element::formatForDebugger() uses operator+=.
> 
> Hum...  I thought paroga had the debug builds fixed.  Maybe his patch didn't land?  /me checks.

Patch is at https://bugs.webkit.org/show_bug.cgi?id=95888, but CQ is to slow :-(
Comment 23 Adam Barth 2012-09-06 00:44:54 PDT
> Patch is at https://bugs.webkit.org/show_bug.cgi?id=95888, but CQ is to slow :-(

Landing now.
Comment 24 Benjamin Poulain 2012-09-06 01:24:13 PDT
(In reply to comment #23)
> > Patch is at https://bugs.webkit.org/show_bug.cgi?id=95888, but CQ is to slow :-(
> 
> Landing now.

Yup, that fixed the bots :)