[Webkit-unassigned] [Bug 91187] [EFL] Add dummy implementation of File system API.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=91187





--- Comment #9 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-09-27 23:48:22 PST ---
(From update of attachment 166141)
View in context: https://bugs.webkit.org/attachment.cgi?id=166141&action=review

>>> Source/WebCore/PlatformEfl.cmake:332
>>> +IF (ENABLE_FILE_SYSTEM)
>> 
>> This condition here is not really needed since the file is already protected by #if ENABLE(FILE_SYSTEM)
> 
> Referring CMakeLists.txt and PlatformEfl.cmake,
> many files are only included when the flag is enabled, even though the files protect using the macro in itself.
> I think that's because it could reduce the compilation time.
> 
> So, I prefer to keep this in my patch.
> 
> What do you think?

I prefer not to guard this in cmake file again. I don't wanna disturb cmake file using excessive ENABLE_ macro. According to my previous build time measurement, there is no build time increase.

> Source/WebCore/platform/efl/AsyncFileSystemEfl.cpp:2
> + * Copyright (C) 2012 Samsung Electronics

Is there any reason to use LGPL ?

>>> Source/WebCore/platform/efl/AsyncFileSystemEfl.cpp:42
>>> +void AsyncFileSystem::openFileSystem(const String& basePath, const String& storageIdentifier, bool, PassOwnPtr<AsyncFileSystemCallbacks> callbacks)
>> 
>> Isn't it generating a lot of warnings all these unused arguments?
> 
> I've ignored these warnings because these will be disappeared by the following patches, and those warnings wouldn't be shown in usual case because FILE_SYSTEM is disabled currently.
> 
> But, for the quality of "this" patch, I will fix this.

I think it would be good if you add variable name when you will use it.

> Source/WebCore/platform/efl/AsyncFileSystemEfl.h:18
> + *

Looks unneeded line.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.