RESOLVED FIXED 51927
Unify string table adding in AtomicString 
https://bugs.webkit.org/show_bug.cgi?id=51927
Summary Unify string table adding in AtomicString
Patrick R. Gansterer
Reported 2011-01-05 07:02:18 PST
Unify string table adding in AtomicString
Attachments
Patch (4.83 KB, patch)
2011-01-05 07:03 PST, Patrick R. Gansterer 		darin: review+
DetailsFormatted DiffDiff
Patch (4.98 KB, patch)
2011-01-08 10:31 PST, Patrick R. Gansterer 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Patrick R. Gansterer
Comment 1 2011-01-05 07:03:32 PST
Created attachment 78001 [details] Patch
Darin Adler
Comment 2 2011-01-06 14:52:45 PST
Comment on attachment 78001 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=78001&action=review > Source/JavaScriptCore/ChangeLog:11 > + * wtf/text/AtomicString.cpp: > + (WTF::addIntoStringTable): > + (WTF::AtomicString::add): > + (WTF::AtomicString::fromUTF8): Change logs are much better when you add some per-function comments. It takes a few more minutes, but is better for the project long-term. > Source/JavaScriptCore/wtf/text/AtomicString.cpp:78 > +static inline PassRefPtr<StringImpl> addIntoStringTable(const T& value) I would suggest calling this “addToStringTable” rather than “addIntoStringTable”.
Patrick R. Gansterer
Comment 3 2011-01-06 14:55:36 PST
(In reply to comment #2) > (From update of attachment 78001 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=78001&action=review > > > Source/JavaScriptCore/ChangeLog:11 > > + * wtf/text/AtomicString.cpp: > > + (WTF::addIntoStringTable): > > + (WTF::AtomicString::add): > > + (WTF::AtomicString::fromUTF8): > > Change logs are much better when you add some per-function comments. It takes a few more minutes, but is better for the project long-term. I usually try to add per function comments, when it make sense. It seams that I need to try it harder. :-) > > > Source/JavaScriptCore/wtf/text/AtomicString.cpp:78 > > +static inline PassRefPtr<StringImpl> addIntoStringTable(const T& value) > > I would suggest calling this “addToStringTable” rather than “addIntoStringTable”. I wasn't very happy with the name too.
Patrick R. Gansterer
Comment 4 2011-01-08 10:31:40 PST
Created attachment 78318 [details] Patch
WebKit Commit Bot
Comment 5 2011-01-08 19:10:29 PST
Comment on attachment 78318 [details] Patch Clearing flags on attachment: 78318 Committed r75343: <http://trac.webkit.org/changeset/75343>
WebKit Commit Bot
Comment 6 2011-01-08 19:10:35 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.