Bug 180545

Summary: Web Inspector: Fix style in remote inspector classes
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, ews-watchlist, inspector-bugzilla-changes, joepeck, keith_miller, mark.lam, msaboff, saam, youennf
Priority: P2 Keywords: DoNotImportToRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
[PATCH] Proposed Fix none

Joseph Pecoraro
Reported 2017-12-07 13:13:26 PST
Fix style in remote inspector classes Make some things `const` and use `final` instead of `override` when possible.
Attachments
[PATCH] Proposed Fix (7.77 KB, patch)
2017-12-07 13:15 PST, Joseph Pecoraro 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Joseph Pecoraro
Comment 1 2017-12-07 13:15:16 PST
Created attachment 328726 [details] [PATCH] Proposed Fix
youenn fablet
Comment 2 2017-12-07 13:18:41 PST
Comment on attachment 328726 [details] [PATCH] Proposed Fix Can you put some of the final methods as private instead of public?
Joseph Pecoraro
Comment 3 2017-12-07 15:59:48 PST
We probably can, but that rarely makes sense to me, especially if it was public in a parent class. I know I've seen this pattern elsewhere though, maybe someone can explain the advantage of that to me.
youenn fablet
Comment 4 2017-12-07 16:12:47 PST
We usually try to restrict the public methods to the minimum. With this pattern, if B derives from A and C is the method being overridden, C can only be called in the context of code using A references/pointers. There may be valid cases for which C can be called from B classes in which case B::C can be made public, but this does not seem to happen a lot.
WebKit Commit Bot
Comment 5 2017-12-07 16:22:59 PST
Comment on attachment 328726 [details] [PATCH] Proposed Fix Clearing flags on attachment: 328726 Committed r225654: <https://trac.webkit.org/changeset/225654>
WebKit Commit Bot
Comment 6 2017-12-07 16:23:00 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.