| Summary: | Add functionality to encode and decode a uint64_t in KeyedCoding | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Woodrow Wang <wwang153> | ||||||||||
| Component: | New Bugs | Assignee: | Woodrow Wang <wwang153> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | bfulgham, cdumez, commit-queue, dbates, ews-watchlist, japhet, mcatanzaro, pvollan, webkit-bug-importer | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | WebKit Nightly Build | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 187773 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Woodrow Wang
2018-08-31 16:59:44 PDT
Created attachment 348689 [details]
Patch
Created attachment 348691 [details]
Patch
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 > +2018-08-31 Woodrow Wang <woodrow_wang@apple.com> Please fix up this file such that it only contains a new change log entry at the top. 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.
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. (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. More specifically, please upload a new patch that addresses all feedback, substitutes Daniel Bates for “NOBODY (OOPS!)” and mark the patch commit-queue ?. Created attachment 348819 [details]
Patch
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. Created attachment 348860 [details]
Patch
Comment on attachment 348860 [details] Patch Clearing flags on attachment: 348860 Committed r235673: <https://trac.webkit.org/changeset/235673> All reviewed patches have been landed. Closing bug. |