Bug 52139 - [EFL] Move cache_directory api
Summary: [EFL] Move cache_directory api
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit EFL (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 55258 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-01-09 21:12 PST by Ryuan Choi
Modified: 2011-03-14 00:11 PDT (History)
8 users (show)

See Also:


Attachments
Patch (7.40 KB, patch)
2011-01-09 21:26 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (7.99 KB, patch)
2011-02-28 08:19 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (8.02 KB, patch)
2011-03-01 18:52 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
Patch (7.88 KB, patch)
2011-03-01 18:54 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff
revised_patch (7.81 KB, patch)
2011-03-12 01:02 PST, Ryuan Choi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryuan Choi 2011-01-09 21:12:35 PST
I think that cache directory is not related to ewk_view.

So, I want to change two cache directory apis like below.
ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]
Comment 1 Ryuan Choi 2011-01-09 21:26:58 PST
Created attachment 78368 [details]
Patch
Comment 2 Gyuyoung Kim 2011-01-10 01:57:33 PST
LGTM :)

BTW, Why don't you move the description to above "@return" ?


/**
 * Return cache directory path.
 *
 * This is guaranteed to be eina_stringshare, so whenever possible
 * save yourself some cpu cycles and use eina_stringshare_ref() instead of 
 * eina_stringshare_add() or strdup().
 *
 * @return cache directory path.
 */
Comment 3 Lucas De Marchi 2011-01-10 04:09:19 PST
(In reply to comment #0)
> I think that cache directory is not related to ewk_view.
> 
> So, I want to change two cache directory apis like below.
> ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]

Looks good, however this breaks the API. Leandro, what do you think about this?
Comment 4 Rafael Antognolli 2011-01-19 08:43:08 PST
(In reply to comment #3)
> (In reply to comment #0)
> > I think that cache directory is not related to ewk_view.
> > 
> > So, I want to change two cache directory apis like below.
> > ewk_view_cache_directory_path_[set|get] -> ewk_settings_cache_directory_path_[set|get]
> 
> Looks good, however this breaks the API. Leandro, what do you think about this?

I think Ryuan is right, cache directory is not related to ewk_view, it's a global setting. Just my 2 cents.
Comment 5 Lukasz Slachciak 2011-02-28 03:36:11 PST
*** Bug 55258 has been marked as a duplicate of this bug. ***
Comment 6 Ryuan Choi 2011-02-28 08:19:48 PST
Created attachment 84063 [details]
Patch
Comment 7 Ryuan Choi 2011-02-28 08:20:41 PST
(In reply to comment #6)
> Created an attachment (id=84063) [details]
> Patch

update lukasz's comment in Bug 55258.
Comment 8 Gyuyoung Kim 2011-03-01 00:18:38 PST
Comment on attachment 84063 [details]
Patch

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

> Source/WebKit/efl/ewk/ewk_settings.cpp:58
> +#endif

Hmm, why don't you move this character constant to 53 line according to alphabet order ? But, when we use macro, I'm not sure if we should adhere alphabet order.
Comment 9 Ryuan Choi 2011-03-01 18:52:31 PST
Created attachment 84352 [details]
Patch
Comment 10 Ryuan Choi 2011-03-01 18:54:29 PST
Created attachment 84354 [details]
Patch
Comment 11 Ryuan Choi 2011-03-01 18:55:24 PST
(In reply to comment #8)
> (From update of attachment 84063 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=84063&action=review
> 
> > Source/WebKit/efl/ewk/ewk_settings.cpp:58
> > +#endif
> 
> Hmm, why don't you move this character constant to 53 line according to alphabet order ? But, when we use macro, I'm not sure if we should adhere alphabet order.

Thank you and I moved it like you mentioned.
Comment 12 WebKit Commit Bot 2011-03-09 04:34:11 PST
Comment on attachment 84354 [details]
Patch

Rejecting attachment 84354 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2

Last 500 characters of output:
ain.cpp
patching file Source/WebKit/efl/ewk/ewk_settings.cpp
Hunk #2 FAILED at 339.
1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_settings.cpp.rej
patching file Source/WebKit/efl/ewk/ewk_settings.h
patching file Source/WebKit/efl/ewk/ewk_view.cpp
Hunk #5 succeeded at 2717 (offset -2 lines).
patching file Source/WebKit/efl/ewk/ewk_view.h

Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christia..." exit_code: 1

Full output: http://queues.webkit.org/results/8114498
Comment 13 Gyuyoung Kim 2011-03-09 04:40:04 PST
(In reply to comment #12)
> (From update of attachment 84354 [details])
> Rejecting attachment 84354 [details] from commit-queue.
> 
> Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=cr-jail-4', 'apply-..." exit_code: 2
> 
> Last 500 characters of output:
> ain.cpp
> patching file Source/WebKit/efl/ewk/ewk_settings.cpp
> Hunk #2 FAILED at 339.
> 1 out of 2 hunks FAILED -- saving rejects to file Source/WebKit/efl/ewk/ewk_settings.cpp.rej
> patching file Source/WebKit/efl/ewk/ewk_settings.h
> patching file Source/WebKit/efl/ewk/ewk_view.cpp
> Hunk #5 succeeded at 2717 (offset -2 lines).
> patching file Source/WebKit/efl/ewk/ewk_view.h
> 
> Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', u'--reviewer', u'Kenneth Rohde Christia..." exit_code: 1
> 
> Full output: http://queues.webkit.org/results/8114498

Hmm, It seems this patch is a little old. Please make this page again on latest trunk.
Comment 14 Ryuan Choi 2011-03-12 01:02:37 PST
Created attachment 85578 [details]
revised_patch
Comment 15 Ryuan Choi 2011-03-12 01:08:24 PST
(In reply to comment #14)
> Created an attachment (id=85578) [details]
> revised_patch

I revised previous patch on latest trunk.
One difference is comment style gyuyoung mentioned above.

Other is same as old one.
Comment 16 WebKit Commit Bot 2011-03-14 00:08:44 PDT
The commit-queue encountered the following flaky tests while processing attachment 85578 [details]:

transitions/interrupted-accelerated-transition.html bug 56242 (authors: simon.fraser@apple.com and tonyg@chromium.org)
The commit-queue is continuing to process your patch.
Comment 17 WebKit Commit Bot 2011-03-14 00:11:33 PDT
Comment on attachment 85578 [details]
revised_patch

Clearing flags on attachment: 85578

Committed r81004: <http://trac.webkit.org/changeset/81004>
Comment 18 WebKit Commit Bot 2011-03-14 00:11:38 PDT
All reviewed patches have been landed.  Closing bug.