[webkit-reviews] review granted: [Bug 179063] Add a FileSystem namespace to FileSystem.cpp : [Attachment 325555] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Christopher Reid
<christopher.reid at am.sony.com>'s request for review:
Bug 179063: Add a FileSystem namespace to FileSystem.cpp
https://bugs.webkit.org/show_bug.cgi?id=179063

Attachment 325555: Patch

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




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

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

I’m not thrilled by this change; I don’t really know what specific name
conflicts this is preventing. But I guess it’s OK.

> Source/WebCore/ChangeLog:12
> +	   * Modules/encryptedmedia/legacy/WebKitMediaKeySession.cpp:

The file list in this change log is a mess. For example, it’s not great to just
leave in all sorts of erroneous "deleted" lines for functions moved inside a
namespace. The point of the change log generation tool is to start the change
log for you. It’s not great to just leave all the lines in there without
looking them over. For a global replace like this, might even want to leave out
all the function names.

> Source/WebCore/Modules/indexeddb/server/IDBServer.cpp:469
> +    Vector<String> entries = WebCore::FileSystem::listDirectory(directory,
ASCIILiteral("*"));

Do we really need the WebCore prefix here? This code is in a namespace nested
inside the WebCore namespace. I’m surprised this is required.