| Summary: | style-check script reports loads of errors on gtk2drawing.c | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Gustavo Noronha (kov) <gustavo> | ||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, cjerdonek, commit-queue, hamaji, mrobinson | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | PC | ||||||
| OS: | OS X 10.5 | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 32833 | ||||||
| Attachments: |
|
||||||
|
Description
Gustavo Noronha (kov)
2010-01-17 09:22:19 PST
Created attachment 46762 [details]
Patch
*** Bug 33609 has been marked as a duplicate of this bug. *** Comment on attachment 46762 [details]
Patch
Looks fine, rs=me
Comment on attachment 46762 [details] Patch Clearing flags on attachment: 46762 Committed r53383: <http://trac.webkit.org/changeset/53383> All reviewed patches have been landed. Closing bug. With this fix, will the style bot still log a single error?
sys.stderr.write('Ignoring %s; This file is exempt from the style guide.\n' % filename)
With files to exempt, I just wanted to clarify whether they should be silently ignored, or if they should be skipped after logging a single line.
(Again, I'm not sure if the style bot is smart enough to distinguish between different types of errors logged to sys.stderr.write.)
(In reply to comment #6) > With this fix, will the style bot still log a single error? The style bot is smart enough to know whether there were any errors. It looks at the exit code to figure out if the check passed. > With files to exempt, I just wanted to clarify whether they should be silently > ignored, or if they should be skipped after logging a single line. I put the logging in to make sure that folks using the tool know that they aren't getting coverage on those files. It will show up in bugzilla if there are other errors caught by the bot, but it won't generate a bug post on its own. (In reply to comment #7) > (In reply to comment #6) > > With this fix, will the style bot still log a single error? > > The style bot is smart enough to know whether there were any errors. It looks > at the exit code to figure out if the check passed. > > > With files to exempt, I just wanted to clarify whether they should be silently > > ignored, or if they should be skipped after logging a single line. > > I put the logging in to make sure that folks using the tool know that they > aren't getting coverage on those files. It will show up in bugzilla if there > are other errors caught by the bot, but it won't generate a bug post on its > own. Okay, thanks, Adam. Just to clarify, then, it seems like there are two classes of file exemptions: (1) exemptions-with-warnings, e.g. files named "gtk2drawing.c" and files in the folder "WebKit/qt/Api/", and (2) exemptions-without-warnings, e.g. all files in the "LayoutTests" folder. If that's correct, I will make an explicit note of this in the code in my next patch. Can you provide a rough rule of thumb as to which type new exemptions should belong to? For example, which type should the "JavaScriptGlue" folder belong to? https://lists.webkit.org/pipermail/webkit-dev/2009-December/011047.html (e-mail referencing JavaScriptGlue) > Okay, thanks, Adam. Just to clarify, then, it seems like there are two classes > of file exemptions: > > (1) exemptions-with-warnings, e.g. files named "gtk2drawing.c" > and files in the folder "WebKit/qt/Api/", and > (2) exemptions-without-warnings, e.g. all files in the "LayoutTests" > folder. Yes. Although the code calls (2) can_handle. > If that's correct, I will make an explicit note of this in the code in my next > patch. Can you provide a rough rule of thumb as to which type new exemptions > should belong to? It's some optimization problem between the principle of least surprise and not spamming useless information. For example, I don't think anyone would be surprised that the LayoutTests aren't covered and if we included a warning that warning would be spammed on every patch. On the other hand, it's kind of surprising that don't have style coverage for gtk2drawing.c and that file isn't modified that often. That makes printing the error a better trade-off. > For example, which type should the "JavaScriptGlue" folder > belong to? I'd print the warning. I've never seen that folder changed so the spam problem is quite low. |