[Webkit-unassigned] [Bug 27551] Make it possible to build JavaScriptCore as shared library without symbol lists

[bugzilla-daemon at webkit.org]

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


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #84999|review?                     |review+
               Flag|                            |




--- Comment #112 from Darin Adler <darin at apple.com>  2011-03-13 18:25:27 PST ---
(From update of attachment 84999)
View in context: https://bugs.webkit.org/attachment.cgi?id=84999&action=review

Concept seems fine and execution seems OK.

The naming is a little inconsistent. We have JS_EXPORTDATA but JS_EXPORT_PRIVATE, so two different styles in how to deal with multiple words.

> Tools/WebKitAPITest/HostWindow.cpp:27
> +#include "config.h"
> +
>  #include "HostWindow.h"

Normally our style is to not have a blank line between the include of config.h and the include of the file's own header. We could have a blank line if the first header is not the file’s own.

>> Tools/WebKitAPITest/config.h:25
>> +#ifndef WebKitAPITests_config_h
> 
> #ifndef header guard has wrong style, please use: WTF_config_h  [build/header_guard] [5]

False error from the style tool. We should file another bug asking for it to be fixed.

> Tools/WebKitAPITest/config.h:31
> +#if USE(EXPORT_MACROS)

Should have a blank line after this since we have a blank line before the #else and the #endif.

> Tools/WebKitAPITest/config.h:41
> +#else /* !USE(EXPORT_MACROS) */

I’m concerned that the boilerplate section of the config files are so long! Seems like plenty of room for error. Is there any way to avoid having so much boilerplate?

>> Source/JavaScriptCore/wtf/Assertions.h:151
>> +WTF_EXPORT_PRIVATE void WTFLog(WTFLogChannel* channel, const char* format, ...) WTF_ATTRIBUTE_PRINTF(2, 3);
> 
> The parameter name "channel" adds no information, so it should be removed.  [readability/parameter_name] [5]

I agree with the style queue about this.

>> Source/JavaScriptCore/wtf/Assertions.h:152
>> +WTF_EXPORT_PRIVATE void WTFLogVerbose(const char* file, int line, const char* function, WTFLogChannel* channel, const char* format, ...) WTF_ATTRIBUTE_PRINTF(5, 6);
> 
> The parameter name "channel" adds no information, so it should be removed.  [readability/parameter_name] [5]

And here too.

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