WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
54733
[EFL] Add coding style exceptions for EFL port
https://bugs.webkit.org/show_bug.cgi?id=54733
Summary
[EFL] Add coding style exceptions for EFL port
Gyuyoung Kim
Reported
2011-02-18 01:34:50 PST
I fix style errors in below ewk files. * ewk/ewk_contextmenu.h * ewk/ewk_cookies.h * ewk/ewk_history.h * ewk/ewk_util.h I'm not sure if this approach is correct. Hello profusion guys, could you let me know your opinions ?
Attachments
Patch
(11.11 KB, patch)
2011-02-18 01:36 PST
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Patch
(2.02 KB, patch)
2011-02-25 20:00 PST
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Proposed Patch
(2.18 KB, patch)
2011-02-25 21:11 PST
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Proposed Patch
(2.18 KB, patch)
2011-02-25 21:24 PST
,
Gyuyoung Kim
no flags
Details
Formatted Diff
Diff
Show Obsolete
(3)
View All
Add attachment
proposed patch, testcase, etc.
Gyuyoung Kim
Comment 1
2011-02-18 01:36:36 PST
Created
attachment 82935
[details]
Patch
Antonio Gomes
Comment 2
2011-02-18 05:23:27 PST
Comment on
attachment 82935
[details]
Patch Is it up for review? LGTM
Gyuyoung Kim
Comment 3
2011-02-20 17:50:06 PST
Comment on
attachment 82935
[details]
Patch I'd like to listen Profusion's guys before requesting review. If you are ok, I'd like to get review for this patch.
Adam Barth
Comment 4
2011-02-21 12:30:49 PST
Comment on
attachment 82935
[details]
Patch Are these files supposed to be in WebKit style or in EFL style? Generally, public APIs in WebKit tend to be in the style of the embedding system.
Lucas De Marchi
Comment 5
2011-02-21 13:17:47 PST
Comment on
attachment 82935
[details]
Patch I really dislike this. This public header follows the EFL coding style and is already being ignored by the bot.
Gyuyoung Kim
Comment 6
2011-02-21 16:07:11 PST
(In reply to
comment #5
)
> (From update of
attachment 82935
[details]
) > I really dislike this. This public header follows the EFL coding style and is already being ignored by the bot.
Hello Lucas, There are many WebKit style errors in ewk header files. Should we ignore the WebKit style errors ?
Gyuyoung Kim
Comment 7
2011-02-24 03:59:38 PST
(In reply to
comment #4
)
> (From update of
attachment 82935
[details]
) > Are these files supposed to be in WebKit style or in EFL style? Generally, public APIs in WebKit tend to be in the style of the embedding system.
EFL port was implemented by EFL style. But, it seems there are conflicts between efl style and webkit style. If efl port keeps current style, can the webkit style checking rule be changed ?
Adam Barth
Comment 8
2011-02-24 14:06:04 PST
> EFL port was implemented by EFL style. But, it seems there are conflicts between efl style and webkit style. If efl port keeps current style, can the webkit style checking rule be changed ?
Yes. If you look at how check-webkit-style works, you'll see similar exceptions for GTK, for example.
Gyuyoung Kim
Comment 9
2011-02-24 16:21:38 PST
(In reply to
comment #8
)
> > EFL port was implemented by EFL style. But, it seems there are conflicts between efl style and webkit style. If efl port keeps current style, can the webkit style checking rule be changed ? > > Yes. If you look at how check-webkit-style works, you'll see similar exceptions for GTK, for example.
Ok, I will check the exceptions for GTK, I think EFL needs to have exceptions rules similar to GTK. I will report this issue in here after finishing my urgent works. :-) Thank you in advance.
Lucas De Marchi
Comment 10
2011-02-25 04:10:33 PST
(In reply to
comment #9
)
> (In reply to
comment #8
) > > > EFL port was implemented by EFL style. But, it seems there are conflicts between efl style and webkit style. If efl port keeps current style, can the webkit style checking rule be changed ? > > > > Yes. If you look at how check-webkit-style works, you'll see similar exceptions for GTK, for example. > > Ok, I will check the exceptions for GTK, I think EFL needs to have exceptions rules similar to GTK. I will report this issue in here after finishing my urgent works. :-) Thank you in advance.
Leandro already had added such exceptions last time I checked. Leandro, what happened?
Leandro Pereira
Comment 11
2011-02-25 06:58:23 PST
(In reply to
comment #10
)
> > Leandro already had added such exceptions last time I checked. > > > Leandro, what happened?
> By a quick inspection (without grep or anything fancier -- I don't have a recent WebKit checkout on this machine), it looks like the CPP checkers were rewritten and the EFL exceptions got lost.
Gyuyoung Kim
Comment 12
2011-02-25 20:00:57 PST
Created
attachment 83920
[details]
Patch I find the exception in checker.py. So, I add additional exception to checker.py. (parameter_name) I need to revert my previous patches.
Bug 54842
,
Bug 54718
,
Bug 54624
,
Bug 54693
. I will revert those after landing this patch.
Gyuyoung Kim
Comment 13
2011-02-25 21:11:24 PST
Created
attachment 83921
[details]
Proposed Patch I add additional efl directories to the exception.
WebKit Review Bot
Comment 14
2011-02-25 21:13:10 PST
Attachment 83921
[details]
did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 Tools/Scripts/webkitpy/style/checker.py:166: trailing whitespace [pep8/W291] [5] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style.
Gyuyoung Kim
Comment 15
2011-02-25 21:24:33 PST
Created
attachment 83925
[details]
Proposed Patch Fix style error.
WebKit Commit Bot
Comment 16
2011-02-26 07:38:29 PST
Comment on
attachment 83925
[details]
Proposed Patch Clearing flags on attachment: 83925 Committed
r79789
: <
http://trac.webkit.org/changeset/79789
>
WebKit Commit Bot
Comment 17
2011-02-26 07:38:36 PST
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug