Bug 167393 - LoadWebLocalizedStrings method should be moved in correct file
Summary: LoadWebLocalizedStrings method should be moved in correct file
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: iPhone / iPad Unspecified
: P2 Normal
Assignee: Aakash Jain
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-24 15:51 PST by Aakash Jain
Modified: 2017-01-25 12:03 PST (History)
3 users (show)

See Also:

Attachments
Proposed patch (5.40 KB, patch)
2017-01-24 18:19 PST, Aakash Jain 		no flags Details | Formatted Diff | Diff
Updated patch (5.57 KB, patch)
2017-01-24 19:03 PST, Aakash Jain 		ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
Updated patch (5.41 KB, patch)
2017-01-25 10:59 PST, Aakash Jain 		commit-queue: commit-queue-
Details | Formatted Diff | Diff
Updated patch (5.42 KB, patch)
2017-01-25 11:01 PST, Aakash Jain 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Aakash Jain 2017-01-24 15:51:36 PST 
Inside WebKit/mac/Misc/WebLocalizableStrings.mm LoadWebLocalizedStrings method is only for iOS (surrounded by #if PLATFORM(IOS)), also this method is only called from a single location in a iOS specific file named WebKit/ios/Misc/WebUIKitSupport.mm.

In order to organize the code better, LoadWebLocalizedStrings() should be moved it iOS specific file WebKit/ios/Misc/WebUIKitSupport.mm
Comment 1 Aakash Jain 2017-01-24 18:19:19 PST 
Created attachment 299660 [details]
Proposed patch
Comment 2 WebKit Commit Bot 2017-01-24 18:22:09 PST 
Attachment 299660 [details] did not pass style-queue:


ERROR: Source/WebKit/ios/Misc/WebUIKitSupport.mm:57:  No space between ^ and block definition.  [whitespace/brackets] [4]
Total errors found: 1 in 3 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 Aakash Jain 2017-01-24 19:03:56 PST 
Created attachment 299664 [details]
Updated patch
Comment 4 Alexey Proskuryakov 2017-01-24 23:12:51 PST 
Comment on attachment 299664 [details]
Updated patch

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

> Source/WebKit/ios/Misc/WebUIKitSupport.mm:48
> +void LoadWebLocalizedStrings(void);

Can this function be a static one too? If made static, it won't need a declaration.

> Source/WebKit/ios/Misc/WebUIKitSupport.mm:66
> +void LoadWebLocalizedStrings(void)

As this is a C++ file (not plain C), "(void)" is not needed, "()" is the right way to write it.
Comment 5 Aakash Jain 2017-01-25 10:59:04 PST 
Created attachment 299715 [details]
Updated patch

Made LoadWebLocalizedStrings static, and removed void.
Comment 6 WebKit Commit Bot 2017-01-25 11:00:13 PST 
Comment on attachment 299715 [details]
Updated patch

Rejecting attachment 299715 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 299715, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/WebKit/ios/ChangeLog contains OOPS!.

Full output: http://webkit-queues.webkit.org/results/2947779
Comment 7 Aakash Jain 2017-01-25 11:01:17 PST 
Created attachment 299716 [details]
Updated patch

Added reviewer name in ChangeLog.
Comment 8 WebKit Commit Bot 2017-01-25 12:03:18 PST 
Comment on attachment 299716 [details]
Updated patch

Clearing flags on attachment: 299716

Committed r211156: <http://trac.webkit.org/changeset/211156>
Comment 9 WebKit Commit Bot 2017-01-25 12:03:21 PST 
All reviewed patches have been landed.  Closing bug.