<html>
    <head>
      <base href="https://bugs.webkit.org/" />
    </head>
    <body>
      <p>
        <div>
            <b><a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [GTK] Initial implementation of JavaScriptCore glib bindings"
   href="https://bugs.webkit.org/show_bug.cgi?id=164061#c5">Comment # 5</a>
              on <a class="bz_bug_link 
          bz_status_NEW "
   title="NEW - [GTK] Initial implementation of JavaScriptCore glib bindings"
   href="https://bugs.webkit.org/show_bug.cgi?id=164061">bug 164061</a>
              from <span class="vcard"><a class="email" href="mailto:mcatanzaro&#64;igalia.com" title="Michael Catanzaro &lt;mcatanzaro&#64;igalia.com&gt;"> <span class="fn">Michael Catanzaro</span></a>
</span></b>
        <pre>Comment on <span class=""><a href="attachment.cgi?id=293040&amp;action=diff" name="attach_293040" title="Patch">attachment 293040</a> <a href="attachment.cgi?id=293040&amp;action=edit" title="Patch">[details]</a></span>
Patch

View in context: <a href="https://bugs.webkit.org/attachment.cgi?id=293040&amp;action=review">https://bugs.webkit.org/attachment.cgi?id=293040&amp;action=review</a>

Just a few initial observations:

<span class="quote">&gt; Source/JavaScriptCore/API/glib/JSCBoolean.cpp:40
&gt; +jsc_boolean_class_init(JSCBooleanClass *)</span >

Keep in mind that you need to follow WebKit coding style in the implementation files:

static void jscBooleanClassInit(JSCBooleanClass*)

The only exception is that API functions of course have to use underscores.

<span class="quote">&gt; Source/JavaScriptCore/API/glib/JSCBoolean.cpp:70
&gt; +jsc_boolean_new(JSCContext *context, gboolean value)</span >

You really can't do anything in these _new functions except call g_object_new, since the _new functions aren't exposed to introspection, the code will never be called from bindings. You want to make sure g_object_new works properly anyway. So everything needs to move to either _init or _constructed.

<span class="quote">&gt; Source/cmake/OptionsGTK.cmake:88
&gt; +WEBKIT_OPTION_DEFINE(ENABLE_JSC_GLIB &quot;Whether to enable support for JavaScriptCore glib bindings.&quot; PUBLIC OFF)</span >

It should be alphabetized.

<span class="quote">&gt; Tools/Scripts/webkitpy/style/checker_unittest.py:289
&gt; +           os.path.join('Source', 'JavaScriptCore', 'API', 'glib', 'JSCBoolean.h'),</span >

This is just a unit test to make sure the style checker works, you shouldn't be modifying it.</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>