Bug 238734 – [WPE] Mark "backend" parameters of web view constructors not nullable

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit API

[Adrian Perez]

In theory according to the documentation NULL is mentioned as a possible
value to use “the default backend” but in practice there has always been
a g_return_value_if_fail(backend, NULL) guarding such usages.

[Adrian Perez]

Created attachment 456564 [details]
Patch

[Martin Robinson]

Comment on attachment 456564 [details]
Patch

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

> Source/WebKit/UIProcess/API/wpe/WebKitWebViewWPE.cpp:65
> + * @backend: (transfer full) (not nullable): the WPE view backend to use.

Hrm. "WPE view backend" seems a bit more ambiguous here than [class@WebKitWebViewBackend]. Is there some reason to use phrases like this rather than the class name?

[Adrian Perez]

(In reply to Martin Robinson from comment #2)
> Comment on attachment 456564 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=456564&action=review
> 
> > Source/WebKit/UIProcess/API/wpe/WebKitWebViewWPE.cpp:65
> > + * @backend: (transfer full) (not nullable): the WPE view backend to use.
> 
> Hrm. "WPE view backend" seems a bit more ambiguous here than
> [class@WebKitWebViewBackend]. Is there some reason to use phrases like this
> rather than the class name?

Yes: the documentation output already writes the type by itself, there is
no need to repeat it every single time. You can find example output here:

  https://people.igalia.com/aperez/Documentation/wpe-webkit-1.1/ctor.WebView.new.html

The documentation comment is expected to *describe* what each parameter
is and/or does, not repeating the type, which the documentation generator
already knows.

[Adrian Perez]

Created attachment 456565 [details]
Screenshot of the generated documentation with patch applied

Hopefully this helps understand how it is unneeded to manually
repeat the type of parameters and returned values, given that
the documentation generator will anyway write them in the output
by itself :-)

[Martin Robinson]

(In reply to Adrian Perez from comment #3)

> Yes: the documentation output already writes the type by itself, there is
> no need to repeat it every single time. You can find example output here:
> 
>  
> https://people.igalia.com/aperez/Documentation/wpe-webkit-1.1/ctor.WebView.
> new.html
> 
> The documentation comment is expected to *describe* what each parameter
> is and/or does, not repeating the type, which the documentation generator
> already knows.

I think the issue is that a phrase like "WPE view backend" doesn't really describe what the parameter does, because it's more of less the type name with spaces in it. As an outsider to WPEWebKit, it is difficult to understand what purpose this parameter serves. It's even a little inaccurate in that a "WPE view backend" isn't the same as a WebKitWebViewBackend, which is a wrapper around a WPE view backend.

[Adrian Perez]

Created attachment 456622 [details]
Patch v2

Now with (hopefully) better wording :)

[EWS]

Committed r292379 (249244@main): <https://commits.webkit.org/249244@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 456622 [details].

[Radar WebKit Bug Importer]

<rdar://problem/91281170>