Bug 95797 – Remove WTF::String::operator+=

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

Remove WTF::String::operator+=

[Adam Barth]

Created attachment 162133 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 162133 [details]
Patch

This looks great, once the other stuff lands.  r- to make the EWS not freak out.

[Adam Barth]

Created attachment 162284 [details]
Patch

[WebKit Review Bot]

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.

[Eric Seidel (no email)]

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. :)

[Adam Barth]

(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.

[Benjamin Poulain]

Looks like I mixed my branches yesterday. I'll make one more patch for Mac. I do that ASAP.

[Adam Barth]

(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!

[Build Bot]

Comment on attachment 162284 [details]
Patch

Attachment 162284 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13765295

[Benjamin Poulain]

(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

[Gyuyoung Kim]

Comment on attachment 162284 [details]
Patch

Attachment 162284 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/13765304

[Adam Barth]

@kangil: Thoughts on the EFL EWS failure?

[Lauro Moura Maranhao Neto]

(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

[Adam Barth]

Ok.  Sounds like I should re-upload this patch and try again.

[Adam Barth]

Created attachment 162344 [details]
re-spin

[WebKit Review Bot]

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.

[Adam Barth]

Thanks for the review!

[Adam Barth]

Comment on attachment 162344 [details]
re-spin

Clearing flags on attachment: 162344

Committed r127702: <http://trac.webkit.org/changeset/127702>

[Adam Barth]

All reviewed patches have been landed.  Closing bug.

(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+=.

[Adam Barth]

> 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.

[Patrick R. Gansterer]

(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 :-(

[Adam Barth]

> Patch is at https://bugs.webkit.org/show_bug.cgi?id=95888, but CQ is to slow :-(

Landing now.

[Benjamin Poulain]

(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 :)