Bug 179696 - [Cocoa] Clean up names of conversion methods after renaming InspectorValue to JSON::Value
Summary: [Cocoa] Clean up names of conversion methods after renaming InspectorValue to...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: BJ Burg
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-11-14 15:03 PST by BJ Burg
Modified: 2017-11-28 15:35 PST (History)
10 users (show)

See Also:


Attachments
Patch (64.48 KB, patch)
2017-11-14 16:46 PST, BJ Burg
no flags Details | Formatted Diff | Diff
Rebased Patch (64.86 KB, patch)
2017-11-28 15:05 PST, BJ Burg
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description BJ Burg 2017-11-14 15:03:45 PST
"This space is intentionally left blank."
Comment 1 Radar WebKit Bug Importer 2017-11-14 15:53:39 PST
<rdar://problem/35544951>
Comment 2 BJ Burg 2017-11-14 16:46:27 PST
Created attachment 326945 [details]
Patch
Comment 3 Build Bot 2017-11-14 16:49:50 PST
This patch modifies the inspector protocol generator. Please ensure that you have rebaselined any generator test results (i.e., by running `Tools/Scripts/run-inspector-generator-tests --reset-results`)
Comment 4 BJ Burg 2017-11-28 15:05:39 PST
Created attachment 327797 [details]
Rebased Patch
Comment 5 Timothy Hatcher 2017-11-28 15:14:31 PST
Comment on attachment 327797 [details]
Rebased Patch

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

Looks fine other than one comment.

> Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_types_implementation.py:115
> -        lines.append('- (instancetype)initWithJSONObject:(RWIProtocolJSONObject *)jsonObject')
> +        lines.append('- (instancetype)initWithProtocolObject:(RWIProtocolJSONObject *)object')
>          lines.append('{')
> -        lines.append('    if (!(self = [super initWithInspectorObject:[jsonObject toInspectorObject].get()]))')
> +        lines.append('    if (!(self = [super initWithJSONObject:[object toJSONObject].get()]))')

This seems odd. Why change [self initWithJSONObject:] to initWithProtocolObject: when super is changing to initWithJSONObject:?

Maybe this method should be initWithProtocolJSONObject:?
Comment 6 BJ Burg 2017-11-28 15:19:41 PST
(In reply to Timothy Hatcher from comment #5)
> Comment on attachment 327797 [details]
> Rebased Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=327797&action=review
> 
> Looks fine other than one comment.
> 
> > Source/JavaScriptCore/inspector/scripts/codegen/generate_objc_protocol_types_implementation.py:115
> > -        lines.append('- (instancetype)initWithJSONObject:(RWIProtocolJSONObject *)jsonObject')
> > +        lines.append('- (instancetype)initWithProtocolObject:(RWIProtocolJSONObject *)object')
> >          lines.append('{')
> > -        lines.append('    if (!(self = [super initWithInspectorObject:[jsonObject toInspectorObject].get()]))')
> > +        lines.append('    if (!(self = [super initWithJSONObject:[object toJSONObject].get()]))')
> 
> This seems odd. Why change [self initWithJSONObject:] to
> initWithProtocolObject: when super is changing to initWithJSONObject:?
> 
> Maybe this method should be initWithProtocolJSONObject:?

I thought it was more sensible to let super be able to use initWithJSONObject rather than keep it in the subclass. Among the two names, I chose the shorter one.
Comment 7 BJ Burg 2017-11-28 15:35:35 PST
Comment on attachment 327797 [details]
Rebased Patch

<https://trac.webkit.org/r225243>