Bug 216339 - Small cleanup in DateTimeFieldElements
Summary: Small cleanup in DateTimeFieldElements
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Aditya Keerthi
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-09-09 19:40 PDT by Aditya Keerthi
Modified: 2020-09-10 07:40 PDT (History)
10 users (show)

See Also:

Attachments
Patch (3.27 KB, patch)
2020-09-09 19:42 PDT, Aditya Keerthi 		no flags Details | Formatted Diff | Diff
Patch (5.14 KB, patch)
2020-09-09 21:12 PDT, Aditya Keerthi 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) 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 Aditya Keerthi 2020-09-09 19:40:10 PDT 
Address some post-review comments after r266779.
Comment 1 Aditya Keerthi 2020-09-09 19:42:44 PDT 
Created attachment 408404 [details]
Patch
Comment 2 Darin Adler 2020-09-09 20:15:07 PDT 
Comment on attachment 408404 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        Removed virtual from overridden methods and reordered declarations for consistency.

You should add override or final to overridden virtual functions, not just remove virtual.
Comment 3 Ryosuke Niwa 2020-09-09 20:21:53 PDT 
What Darin said.
Comment 4 Aditya Keerthi 2020-09-09 20:46:59 PDT 
The classes are all final, is final still required?

Following the comment here: https://bugs.webkit.org/show_bug.cgi?id=215155#c20.
Comment 5 Ryosuke Niwa 2020-09-09 21:02:31 PDT 
(In reply to Aditya Keerthi from comment #4)
> The classes are all final, is final still required?
> 
> Following the comment here:
> https://bugs.webkit.org/show_bug.cgi?id=215155#c20.

Yes. https://webkit.org/code-style-guidelines/#override-methods
Comment 6 Aditya Keerthi 2020-09-09 21:12:01 PDT 
Created attachment 408410 [details]
Patch
Comment 7 EWS 2020-09-10 07:39:17 PDT 
Committed r266824: <https://trac.webkit.org/changeset/266824>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408410 [details].
Comment 8 Radar WebKit Bug Importer 2020-09-10 07:40:18 PDT 
<rdar://problem/68642387>