<html>
<head>
<base href="https://bugs.webkit.org/" />
</head>
<body><span class="vcard"><a class="email" href="mailto:benjamin@webkit.org" title="Benjamin Poulain <benjamin@webkit.org>"> <span class="fn">Benjamin Poulain</span></a>
</span> changed
<a class="bz_bug_link
bz_status_NEW "
title="NEW - [INTL] Implement Intl.Collator.prototype.resolvedOptions ()"
href="https://bugs.webkit.org/show_bug.cgi?id=147601">bug 147601</a>
<br>
<table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>What</th>
<th>Removed</th>
<th>Added</th>
</tr>
<tr>
<td style="text-align:right;">Attachment #262678 Flags</td>
<td>review?
</td>
<td>review+, commit-queue-
</td>
</tr></table>
<p>
<div>
<b><a class="bz_bug_link
bz_status_NEW "
title="NEW - [INTL] Implement Intl.Collator.prototype.resolvedOptions ()"
href="https://bugs.webkit.org/show_bug.cgi?id=147601#c7">Comment # 7</a>
on <a class="bz_bug_link
bz_status_NEW "
title="NEW - [INTL] Implement Intl.Collator.prototype.resolvedOptions ()"
href="https://bugs.webkit.org/show_bug.cgi?id=147601">bug 147601</a>
from <span class="vcard"><a class="email" href="mailto:benjamin@webkit.org" title="Benjamin Poulain <benjamin@webkit.org>"> <span class="fn">Benjamin Poulain</span></a>
</span></b>
<pre>Comment on <span class=""><a href="attachment.cgi?id=262678&action=diff" name="attach_262678" title="Patch">attachment 262678</a> <a href="attachment.cgi?id=262678&action=edit" title="Patch">[details]</a></span>
Patch
View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=262678&action=review">https://bugs.webkit.org/attachment.cgi?id=262678&action=review</a>
I did not find any error. Some minor comments:
<span class="quote">> Source/JavaScriptCore/ChangeLog:3
> + [INTL] Implement Intl.Collator.prototype.resolvedOptions ()</span >
Don't forget to add your copyright on the files with significant changes.
<span class="quote">> Source/JavaScriptCore/runtime/IntlCollator.h:74
> + bool m_numeric;
> + String m_sensitivity;
> + bool m_ignorePunctuation;</span >
Let's move the booleans together to improve packing a bit.
<span class="quote">> Source/JavaScriptCore/runtime/IntlCollatorPrototype.cpp:123
> // The function returns a new object whose properties and attributes are set as if constructed by an object literal assigning to each of the following properties the value of the corresponding internal slot of this Collator object (see 10.4): locale, usage, sensitivity, ignorePunctuation, collation, as well as those properties shown in Table 1 whose keys are included in the %Collator%[[relevantExtensionKeys]] internal slot of the standard built-in object that is the initial value of Intl.Collator.</span >
Can you please break this comment over multiple lines?
<span class="quote">> Source/JavaScriptCore/runtime/IntlObject.cpp:631
> + String locale, noExtensionsLocale;</span >
Please move noExtensionsLocale to its own line. One statement per declaration.
<span class="quote">> Source/JavaScriptCore/runtime/IntlObject.cpp:646
> + ASSERT(extensionIndex != notFound);</span >
Let's make that a RELEASE_ASSERT().
I don't like the idea of a notFound accidentally being used for length computation.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>