[webkit-reviews] review denied: [Bug 45395] GObject Introspection enhancements : [Attachment 66904] gobject introspection enhancements

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has denied  review:
Bug 45395: GObject Introspection enhancements
https://bugs.webkit.org/show_bug.cgi?id=45395

Attachment 66904: gobject introspection enhancements
https://bugs.webkit.org/attachment.cgi?id=66904&action=review

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
> Index: GNUmakefile.am
> ===================================================================
> --- GNUmakefile.am	(revisión: 66973)
> +++ GNUmakefile.am	(copia de trabajo)
> @@ -397,7 +397,7 @@
>  WEBKIT_GIRSOURCES += WebKit- at WEBKITGTK_API_VERSION@.gir
>  
>  $(WEBKIT_GIRSOURCES): $(G_IR_SCANNER) $(JSCORE_GIRSOURCES)
libwebkitgtk- at WEBKITGTK_API_MAJOR_VERSION@. at WEBKITGTK_API_MINOR_VERSION@.la
> -	$(AM_V_GEN)$(G_IR_SCANNER) -v --namespace WebKit
--nsversion=@WEBKITGTK_API_VERSION@ \
> +	$(AM_V_GEN)$(G_IR_SCANNER) -v --symbol-prefix=webkit --namespace WebKit
--nsversion=@WEBKITGTK_API_VERSION@ \

Is this needed on top of the --prefix parameter?

>	     --include=GObject-2.0 \
>	     --include=Gtk- at GTK_API_VERSION@ \
>	     --include=JSCore- at WEBKITGTK_API_VERSION@ \



>   */
>  gchar* webkit_web_frame_get_inner_text(WebKitWebFrame* frame)
>  {
> @@ -790,7 +790,7 @@
>   * webkit_web_frame_dump_render_tree:
>   * @frame: a #WebKitWebFrame
>   *
> - * Return value: Non-recursive render tree dump of @frame
> + * Return value: (transfer none): Non-recursive render tree dump of @frame
>   */

This is not public API, and besides the annotation is wrong (?).

>  gchar* webkit_web_frame_dump_render_tree(WebKitWebFrame* frame)
>  {
> @@ -814,7 +814,7 @@
>   * @frame: a #WebKitWebFrame
>   * @id: an element ID string
>   *
> - * Return value: The counter value of element @id in @frame
> + * Return value: (transfer none): The counter value of element @id in @frame

>   */

Same thing...

>   */
>  int webkit_web_frame_number_of_pages(WebKitWebFrame* frame, float pageWidth,
float pageHeight)
>  {
> @@ -877,7 +877,7 @@
>   * webkit_web_frame_get_pending_unload_event_count:
>   * @frame: a #WebKitWebFrame
>   *
> - * Return value: number of pending unload events
> + * Return value: (transfer none): number of pending unload events
>   */

Transfer annotation for int? This is not needed, same for booleans and any non
pointer type.

>  }
>  
>  /**
> - * webkit_network_response_get_soup_message:
> + * webkit_network_response_get_message:

You are most definitely not allowed to change the function name. Not sure why
are you doing that but you can't.

Please go again over all annotations, a lot of them are wrong.


> Index: WebCore/bindings/scripts/CodeGeneratorGObject.pm
> ===================================================================
> --- WebCore/bindings/scripts/CodeGeneratorGObject.pm	(revisión: 66983)
> +++ WebCore/bindings/scripts/CodeGeneratorGObject.pm	(copia de trabajo)

Wow, sorry but no, this is way too ugly :). If the scanner can't cope with our
names as they are we'll have to teach it to, I won't accept this patch.

Besides, I thought we had decided to write the introspection data in a
different file and include it at compile time? I don't think you can possibly
get right all the stuff automatically?

r- for all the comments above.