Bug 152156

Summary:

check-for-inappropriate-objc-class-names should check all class names, not just externally visible ones

Product:

WebKit

Reporter:

Joseph Pecoraro <joepeck>

Component:

Tools / Tests

Assignee:

Joseph Pecoraro <joepeck>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, joepeck, lforschler, mitz

Priority:

Version:

Other

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
[PATCH] Proposed Fix	mitz: review+
[PATCH] Proposed Fix	joepeck: commit-queue-
[PATCH] Proposed Fix	joepeck: commit-queue-
[PATCH] Proposed Fix	none

Joseph Pecoraro

Reported 2015-12-10 17:20:48 PST

* SUMMARY check-for-inappropriate-objc-class-names should check all class names, not just externally visible ones. The script currently uses `nm -Ugjp` where `-g` filters the list of symbols down to "only global (external) symbols". Our frameworks should be clean and identify all ObjC classes with appropriately prefixed names, not just external symbols. This also helps with code readability.

Attachments
[PATCH] Proposed Fix (7.60 KB, patch) 2015-12-10 17:23 PST, Joseph Pecoraro	mitz: review+	Details Formatted Diff Diff
[PATCH] Proposed Fix (13.31 KB, patch) 2015-12-10 19:28 PST, Joseph Pecoraro	joepeck: commit-queue-	Details Formatted Diff Diff
[PATCH] Proposed Fix (13.33 KB, patch) 2015-12-10 19:33 PST, Joseph Pecoraro	joepeck: commit-queue-	Details Formatted Diff Diff
[PATCH] Proposed Fix (13.33 KB, patch) 2015-12-10 19:52 PST, Joseph Pecoraro	no flags	Details Formatted Diff Diff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.

Joseph Pecoraro

Comment 1 2015-12-10 17:23:30 PST

Created attachment 267143 [details] [PATCH] Proposed Fix

mitz

Comment 2 2015-12-10 17:39:47 PST

Comment on attachment 267143 [details] [PATCH] Proposed Fix Before you commit this, please verify that it doesn’t break Apple-internal projects that use this script.

Joseph Pecoraro

Comment 3 2015-12-10 17:56:03 PST

I did check other projects, however I clearly missed iOS builds! I will fix iOS.

Joseph Pecoraro

Comment 4 2015-12-10 19:28:17 PST

Created attachment 267150 [details] [PATCH] Proposed Fix Cover a few iOS cases I missed.

WebKit Commit Bot

Comment 5 2015-12-10 19:29:22 PST

Attachment 267150 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:35: _WKOneShotDisplayLinkHandler is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

Joseph Pecoraro

Comment 6 2015-12-10 19:33:27 PST

Created attachment 267152 [details] [PATCH] Proposed Fix

WebKit Commit Bot

Comment 7 2015-12-10 19:34:40 PST

Attachment 267152 [details] did not pass style-queue: ERROR: Source/WebKit2/UIProcess/mac/RemoteLayerTreeDrawingAreaProxy.h:35: _WKOneShotDisplayLinkHandler is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.

mitz

Comment 8 2015-12-10 19:36:00 PST

No need for the underscore prefix on internal classes. Only some private identifiers need to be underscore-prefixed.

Joseph Pecoraro

Comment 9 2015-12-10 19:52:35 PST

Created attachment 267154 [details] [PATCH] Proposed Fix

WebKit Commit Bot

Comment 10 2015-12-11 12:56:35 PST

Comment on attachment 267154 [details] [PATCH] Proposed Fix Clearing flags on attachment: 267154 Committed r193972: <http://trac.webkit.org/changeset/193972>

WebKit Commit Bot

Comment 11 2015-12-11 12:56:39 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.