Bug 65305 - [EFL] Move ewk_view API documentation to the header file
Summary: [EFL] Move ewk_view API documentation to the header file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: All Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-28 00:18 PDT by Grzegorz Czajkowski
Modified: 2011-08-01 03:02 PDT (History)
6 users (show)

See Also:


Attachments
proposed patch (105.45 KB, patch)
2011-07-28 00:23 PDT, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff
updated patch (105.32 KB, patch)
2011-07-28 06:34 PDT, Grzegorz Czajkowski
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Grzegorz Czajkowski 2011-07-28 00:18:35 PDT
There are three arguments for this:
1. Dev package will contain all API description so developers
   using webkit-efl-dev package don't need to check source code of cpp
   files when they want to know how to use the given API.
2. Doxygen might generate documentation just using webkit-ef-dev package
   so there will be no need to have WebKit sources.
3. It will be consistent with structures descriptions.


This patch only moves API documentation, it doesn't modify anything else.

If you agree with this idea I will prepare another patches for other ewk files soon.
Comment 1 Grzegorz Czajkowski 2011-07-28 00:23:08 PDT
Created attachment 102232 [details]
proposed patch
Comment 2 Raphael Kubo da Costa (:rakuco) 2011-07-28 05:04:53 PDT
The patch itself looks OK to me, but I have a few comments:

(In reply to comment #0)
> There are three arguments for this:
> 1. Dev package will contain all API description so developers
>    using webkit-efl-dev package don't need to check source code of cpp
>    files when they want to know how to use the given API.
> 2. Doxygen might generate documentation just using webkit-ef-dev package
>    so there will be no need to have WebKit sources.

Mentioning a package by name like that assumes it will always have that name in all distributions and whatnot. Personally, I would be more generic and just say that it is easier to check the apidox in the header, as people using webkit-efl from packages probably won't have the C files with them and the doxygen documentation can be generated from the headers without the need for the whole source files.

> If you agree with this idea I will prepare another patches for other ewk files soon.

Do you mean that there are other patches which move documentation in ewk? If so, wouldn't it be better to do that all at once?
Comment 3 Grzegorz Czajkowski 2011-07-28 05:19:58 PDT
(In reply to comment #2)
> The patch itself looks OK to me, but I have a few comments:
> 
> (In reply to comment #0)
> > There are three arguments for this:
> > 1. Dev package will contain all API description so developers
> >    using webkit-efl-dev package don't need to check source code of cpp
> >    files when they want to know how to use the given API.
> > 2. Doxygen might generate documentation just using webkit-ef-dev package
> >    so there will be no need to have WebKit sources.
> 
> Mentioning a package by name like that assumes it will always have that name in all distributions and whatnot. Personally, I would be more generic and just say that it is easier to check the apidox in the header, as people using webkit-efl from packages probably won't have the C files with them and the doxygen documentation can be generated from the headers without the need for the whole source files.

Ok, I will fix a description.

> 
> > If you agree with this idea I will prepare another patches for other ewk files soon.
> 
> Do you mean that there are other patches which move documentation in ewk? If so, wouldn't it be better to do that all at once?

Yes, there are, but I wanted to avoid that huge patch. Besides currently only ewk_view is adjusted to EFL's style. If other files are ready I will move documentation for them. I suggest to submit three patches for this:
1. ewk_view
2. ewk_frame
3. other ewk_files
Is it ok?
Comment 4 Raphael Kubo da Costa (:rakuco) 2011-07-28 05:43:43 PDT
If it's just moving comments around, I wouldn't mind a big patch that does it all. But if you prefer to split it, I'm fine with that too.
Comment 5 Grzegorz Czajkowski 2011-07-28 06:34:48 PDT
Created attachment 102253 [details]
updated patch

fix description in ChangeLog
Comment 6 Raphael Kubo da Costa (:rakuco) 2011-07-28 06:50:28 PDT
r+ from my side.
Comment 7 Gyuyoung Kim 2011-07-29 01:58:51 PDT
Comment on attachment 102253 [details]
updated patch

LGTM
Comment 8 Gyuyoung Kim 2011-07-29 02:00:33 PDT
Hello tkent. Could you please review this patch ? Sorry for adding CC. I think you can review this.
Comment 9 WebKit Review Bot 2011-07-29 02:59:22 PDT
Comment on attachment 102253 [details]
updated patch

Clearing flags on attachment: 102253

Committed r91981: <http://trac.webkit.org/changeset/91981>
Comment 10 WebKit Review Bot 2011-07-29 02:59:27 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Grzegorz Czajkowski 2011-08-01 03:00:06 PDT
Hello tkent,

I've added bug which moves remaining API documentaion to the header files. Could you review this patch, please? https://bugs.webkit.org/show_bug.cgi?id=65305

It may avoid merge conflicts if the patch will be commited asap. Thanks.
Comment 12 Grzegorz Czajkowski 2011-08-01 03:02:16 PDT
(In reply to comment #11)
> Hello tkent,
> 
> I've added bug which moves remaining API documentaion to the header files. Could you review this patch, please? https://bugs.webkit.org/show_bug.cgi?id=65305
> 
> It may avoid merge conflicts if the patch will be commited asap. Thanks.

I am sorry for mistake, correct URL of bug https://bugs.webkit.org/show_bug.cgi?id=65373