Bug 51927 – Unify string table adding in AtomicString

• Component: New Bugs
• Status: RESOLVED FIXED

[Patrick R. Gansterer]

Unify string table adding in AtomicString

[Patrick R. Gansterer]

Created attachment 78001 [details]
Patch

[Darin Adler]

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]

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

Created attachment 78318 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 78318 [details]
Patch

Clearing flags on attachment: 78318

Committed r75343: <http://trac.webkit.org/changeset/75343>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.