Summary: | Add a script to check for platform layering violations | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Carlos Garcia Campos <cgarcia> | ||||||
Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | darin | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 140288 | ||||||||
Bug Blocks: | 21354 | ||||||||
Attachments: |
|
Description
Carlos Garcia Campos
2015-01-08 08:20:46 PST
Created attachment 244259 [details]
Patch
Could you post a copy of the current output of the script? (In reply to comment #2) > Could you post a copy of the current output of the script? Sure, I'll attach it. Created attachment 244261 [details]
Current output
I think there are a few false positives, that are not layering violations, but that should be fixed anyway using <> instead of "" for example.
(In reply to comment #4) > I think there are a few false positives, that are not layering violations, > but that should be fixed anyway using <> instead of "" for example. Before we start fixing the actual violations, I think it’s important to wipe out the false positives. Could you give an example of one where you think using <> is the right thing to do? OK, <stdio.h> clearly falls into that category. But I’m more worried about the "TextCodec.cpp" one, for example. Why is that showing up as a violation? I love the idea of the script, and I think I’ll say review+, but I really want this script in a place where its output is actionable as soon as possible, maybe even before you land it? (In reply to comment #6) > OK, <stdio.h> clearly falls into that category. Yes, exactly. > But I’m more worried about the "TextCodec.cpp" one, for example. Why is that > showing up as a violation? hmm, that's a bug in the script itself, I was assuming all includes are header files, I'll fix that. > I love the idea of the script, and I think I’ll say review+, but I really > want this script in a place where its output is actionable as soon as > possible, maybe even before you land it? Sure, I'll check and fix all the false positives before, thanks! Committed r178167: <http://trac.webkit.org/changeset/178167> (In reply to comment #8) > Committed r178167: <http://trac.webkit.org/changeset/178167> False positives were fixed in r178166 and I fixed the cpp includes before landing this. |