[webkit-reviews] review granted: [Bug 217390] Refactor TestOptions code in WebKitTestRunner to make it easier to rationalize and extend : [Attachment 410649] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 217390: Refactor TestOptions code in WebKitTestRunner to make it easier to
rationalize and extend
https://bugs.webkit.org/show_bug.cgi?id=217390

Attachment 410649: Patch

https://bugs.webkit.org/attachment.cgi?id=410649&action=review




--- Comment #2 from Darin Adler <darin at apple.com> ---
Comment on attachment 410649
  --> https://bugs.webkit.org/attachment.cgi?id=410649
Patch

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

I approve the idea of using the standard library in our test tools. However I
don’t know what specifically motivates doing more of that here while still
having so much code using WTF.

> Tools/WebKitTestRunner/Options.cpp:116
> +static bool parseFeature(String featureString,
std::unordered_map<std::string, bool>& features)

Funny that this takes String, not StringView or const String&

> Tools/WebKitTestRunner/TestOptions.cpp:101
> +enum class KeyType {

Perhaps : uint8_t?