Bug 106413 - The word "selector" is somewhat redundant redundantly used in SelectorChecker.
Summary: The word "selector" is somewhat redundant redundantly used in SelectorChecker.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Dimitri Glazkov (Google)
URL:
Keywords:
Depends on:
Blocks: 89879
  Show dependency tree / graph
 
Reported: 2013-01-08 21:02 PST by Dimitri Glazkov (Google)
Modified: 2013-01-10 22:20 PST (History)
8 users (show)

See Also:

Attachments
Patch (17.46 KB, patch)
2013-01-08 21:05 PST, Dimitri Glazkov (Google) 		no flags Details | Formatted Diff | Diff
Patch for landing (17.46 KB, patch)
2013-01-10 21:40 PST, Dimitri Glazkov (Google) 		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 Dimitri Glazkov (Google) 2013-01-08 21:02:48 PST 
The word "selector" is somewhat redundant redundantly used in SelectorChecker.
Comment 1 Dimitri Glazkov (Google) 2013-01-08 21:05:15 PST 
Created attachment 181837 [details]
Patch
Comment 2 Build Bot 2013-01-08 21:18:57 PST 
Comment on attachment 181837 [details]
Patch

Attachment 181837 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15754580
Comment 3 Build Bot 2013-01-08 21:19:55 PST 
Comment on attachment 181837 [details]
Patch

Attachment 181837 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15762353
Comment 4 Antti Koivisto 2013-01-09 03:38:14 PST 
Comment on attachment 181837 [details]
Patch

LGTM, not sure why it is not building on Mac.
Comment 5 Dimitri Glazkov (Google) 2013-01-09 09:47:25 PST 
(In reply to comment #4)
> LGTM, not sure why it is not building on Mac.

will fix.
Comment 6 Dimitri Glazkov (Google) 2013-01-09 20:04:25 PST 
It's weird. The error makes it sound like there's a macro check(X) that's defined somewhere.
Comment 7 Antti Koivisto 2013-01-10 00:51:03 PST 
/usr/include/AssertMacros.h

#ifndef __ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES
	/* If we haven't set this yet, it defaults to on.  In the next release, this will default to off. */
	#define	__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES	1
#endif

#if	__ASSERT_MACROS_DEFINE_VERSIONS_WITHOUT_UNDERSCORES

	#ifndef check
	#define check(assertion)  __Check(assertion)
	#endif
...

Lame.

Hmm, how about naming them match() instead? Might even read better.
Comment 8 Allan Sandfeld Jensen 2013-01-10 01:32:19 PST 
Does really improve the naming? I admit SelectorChecker::checkSelector is a _very_ redundant (even Checker::check() is), but I like that the method name in itself is fully descriptive.
Comment 9 Dimitri Glazkov (Google) 2013-01-10 21:39:50 PST 
(In reply to comment #7)
> 
> Hmm, how about naming them match() instead? Might even read better.

Sounds good, going with that.
Comment 10 Dimitri Glazkov (Google) 2013-01-10 21:40:10 PST 
Created attachment 182262 [details]
Patch for landing
Comment 11 WebKit Review Bot 2013-01-10 22:19:58 PST 
Comment on attachment 182262 [details]
Patch for landing

Clearing flags on attachment: 182262

Committed r139406: <http://trac.webkit.org/changeset/139406>
Comment 12 WebKit Review Bot 2013-01-10 22:20:02 PST 
All reviewed patches have been landed.  Closing bug.