Bug 189216 – Add functionality to encode and decode a uint64_t in KeyedCoding

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Woodrow Wang]

Added functionality to encode and decode a uint64_t in KeyedDecoding

[Woodrow Wang]

Created attachment 348689 [details]
Patch

[Woodrow Wang]

Created attachment 348691 [details]
Patch

[Daniel Bates]

Comment on attachment 348691 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Added functionality to encode and decode a uint64_t in KeyedCoding. 

This line should be a verbatim copy of the bug title. Please remove the period.

> Source/WebCore/ChangeLog:7
>  

Please explain why you are adding this functionality given that this patch just adds infrastructure and does not make use of it.

> Source/WebCore/ChangeLog:22
> [phone]  Woodrow Wang  <[e-mail]>

Please fix up this file such that it only contains a new change log entry at the top.

[Michael Catanzaro]

Comment on attachment 348691 [details]
Patch

The changes to KeyedEncoderGlib look fine to me.

I agree that, if this is going to land in its own patch, there should be an explanation of how this functionality will be imminently used in a follow-up patch.

[Woodrow Wang]

I've added this functionality in order to be able to encode and decode the raw uint64_t representation of an OptionSet for my patch here https://bugs.webkit.org/show_bug.cgi?id=187773 

The changes in the KeyedEncoder/KeyedDecoder for Glib were made because they are derived classes of KeyedCoding which contains pure virtual functions that need to be implemented.

[Daniel Bates]

(In reply to Woodrow Wang from comment #5)
> I've added this functionality in order to be able to encode and decode the
> raw uint64_t representation of an OptionSet for my patch here
> https://bugs.webkit.org/show_bug.cgi?id=187773 
> 
> The changes in the KeyedEncoder/KeyedDecoder for Glib were made because they
> are derived classes of KeyedCoding which contains pure virtual functions
> that need to be implemented.

Great! Please explain this in the description portion of the ChangeLog entry: under the Reviewed by line.

[Daniel Bates]

More specifically, please upload a new patch that addresses all feedback, substitutes Daniel Bates for “NOBODY (OOPS!)” and mark the patch commit-queue ?.

[Woodrow Wang]

Created attachment 348819 [details]
Patch

[Daniel Bates]

Comment on attachment 348819 [details]
Patch

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

> Source/WebCore/ChangeLog:9
> +        representation of an OptionSet for my patch here https://bugs.webkit.org/show_bug.cgi?id=187773 

I would put the URL in angle brackets and add a period at the end of this sentence.

[Woodrow Wang]

Created attachment 348860 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 348860 [details]
Patch

Clearing flags on attachment: 348860

Committed r235673: <https://trac.webkit.org/changeset/235673>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/44142988>