Bug 66293 – [EFL] Fix and update doxygen documentation for ewk_view

• Component: WebKit EFL
• Status: RESOLVED FIXED

[Grzegorz Czajkowski]

This patch
- fixes method descriptions,
- adds briefs for structure and typdef,
- removes dots from descriptions of parameter and return command (see EFL's documentation),
- move includes and defines below a brief of ewk_view.h.

[Grzegorz Czajkowski]

Created attachment 104029 [details]
proposed patch

[Leandro Pereira]

Could you please try splitting this into multiple patches?

[Grzegorz Czajkowski]

(In reply to comment #2)
> Could you please try splitting this into multiple patches?

I agree with you that this patch is huge. What is your proposition to splitting this patch? What will you say about:

patch #1 - changes in ewk_view.cpp + ChangeLog
patch #2 - changes in ewk_view.h (typedefs and structs description) + ChangeLog
patch #3 - API documentation part 1 + ChangeLog (ewk_view.h)
patch #4 - API documentation part 2 (settings) + ChangeLog (ewk_view.h)

These patches will be related with the same bug, they will have the same ChangeLog.

[Leandro Pereira]

(In reply to comment #3)
> (In reply to comment #2)
> > Could you please try splitting this into multiple patches?
> 
> I agree with you that this patch is huge. What is your proposition to splitting this patch? What will you say about:
> 
> patch #1 - changes in ewk_view.cpp + ChangeLog
> patch #2 - changes in ewk_view.h (typedefs and structs description) + ChangeLog
> patch #3 - API documentation part 1 + ChangeLog (ewk_view.h)
> patch #4 - API documentation part 2 (settings) + ChangeLog (ewk_view.h)
> 
> These patches will be related with the same bug, they will have the same 
> ChangeLog.
>

Looks like a plan.

[Grzegorz Czajkowski]

(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > Could you please try splitting this into multiple patches?
> > 
> > I agree with you that this patch is huge. What is your proposition to splitting this patch? What will you say about:
> > 
> > patch #1 - changes in ewk_view.cpp + ChangeLog
> > patch #2 - changes in ewk_view.h (typedefs and structs description) + ChangeLog
> > patch #3 - API documentation part 1 + ChangeLog (ewk_view.h)
> > patch #4 - API documentation part 2 (settings) + ChangeLog (ewk_view.h)
> > 
> > These patches will be related with the same bug, they will have the same 
> > ChangeLog.
> >
> 
> Looks like a plan.

It's only proposition :) I didn't split a patch into smaller ones before. I don't know what are general rules for that. Have you ever experienced similar case?

[Gyuyoung Kim]

Hello Kent,

First of all, sorry for adding CC. I have a question.

Grzegorz refines descriptions of ewk_view's APIs. However, this patch is a little huge. I'm not sure if we should split this patch into smaller ones. Should this patch be splited into more smaller patches ?

[Kent Tamura]

(In reply to comment #6)
> Hello Kent,
> 
> First of all, sorry for adding CC. I have a question.
> 
> Grzegorz refines descriptions of ewk_view's APIs. However, this patch is a little huge. I'm not sure if we should split this patch into smaller ones. Should this patch be splited into more smaller patches ?

We should reduce the patch size as possible in general.
It seems we can split this patch easily.

[Gyuyoung Kim]

(In reply to comment #7)
> (In reply to comment #6)
> > Hello Kent,
> > 
> > First of all, sorry for adding CC. I have a question.
> > 
> > Grzegorz refines descriptions of ewk_view's APIs. However, this patch is a little huge. I'm not sure if we should split this patch into smaller ones. Should this patch be splited into more smaller patches ?
> 
> We should reduce the patch size as possible in general.
> It seems we can split this patch easily.

Ok, I will guide we should not make huge patch regardless patch is major or minor in future.

[Grzegorz Czajkowski]

(In reply to comment #7)
> (In reply to comment #6)
> > Hello Kent,
> > 
> > First of all, sorry for adding CC. I have a question.
> > 
> > Grzegorz refines descriptions of ewk_view's APIs. However, this patch is a little huge. I'm not sure if we should split this patch into smaller ones. Should this patch be splited into more smaller patches ?
> 
> We should reduce the patch size as possible in general.
> It seems we can split this patch easily.

Do you mean that I should split the patch into the few bugs? Or should I attach multiple patches to this bug? If second what to do with ChageLog? Does the same ChangeLog should be attached to each patch? (the most changes are included in ewk_view.h)

[Kent Tamura]

(In reply to comment #9)
> Do you mean that I should split the patch into the few bugs? Or should I attach multiple patches to this bug? If second what to do with ChageLog? Does the same ChangeLog should be attached to each patch? (the most changes are included in ewk_view.h)

Both are ok.  But many reviewers prefer one patch per one bug.
Each patch should have its ChangeLog anyway.

[Grzegorz Czajkowski]

Created attachment 104497 [details]
patch  #1 (changes in ewk_view.cpp)

[Grzegorz Czajkowski]

Created attachment 104499 [details]
patch  #2 (changes in ewk_view.h) - typedefs and structs descriptions

[Grzegorz Czajkowski]

Created attachment 104500 [details]
patch  #3 (changes in ewk_view.h) - API descriptions part1

[Grzegorz Czajkowski]

Created attachment 104501 [details]
patch  #4 (changes in ewk_view.h) - API descriptions part2

[WebKit Review Bot]

Comment on attachment 104497 [details]
patch  #1 (changes in ewk_view.cpp)

Clearing flags on attachment: 104497

Committed r93401: <http://trac.webkit.org/changeset/93401>

[WebKit Review Bot]

Comment on attachment 104499 [details]
patch  #2 (changes in ewk_view.h) - typedefs and structs descriptions

Clearing flags on attachment: 104499

Committed r93485: <http://trac.webkit.org/changeset/93485>

[WebKit Review Bot]

Comment on attachment 104500 [details]
patch  #3 (changes in ewk_view.h) - API descriptions part1

Clearing flags on attachment: 104500

Committed r93486: <http://trac.webkit.org/changeset/93486>

[WebKit Review Bot]

Comment on attachment 104501 [details]
patch  #4 (changes in ewk_view.h) - API descriptions part2

Clearing flags on attachment: 104501

Committed r93488: <http://trac.webkit.org/changeset/93488>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.