Bug 38169 - check-webkit-style returns non-zero when patch is entirely minus lines
Summary: check-webkit-style returns non-zero when patch is entirely minus lines
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
: 39690 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-04-26 22:25 PDT by Eric Seidel (no email)
Modified: 2010-08-05 06:42 PDT (History)
6 users (show)

See Also:


Attachments
Proposed Patch (4.08 KB, patch)
2010-08-04 01:17 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff
Proposed Patch V2 (7.06 KB, patch)
2010-08-04 02:52 PDT, Kenichi Ishibashi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2010-04-26 22:25:59 PDT
check-webkit-style returns non-zero when patch is entirely minus lines

Total errors found: 0 in 0 files

This is caused by PatchChecker.check which ignores non-add/modify lines.
It should at least notice the lines and count the file, even if it doesn't say there are errors there.
Comment 1 Chris Jerdonek 2010-04-26 23:41:51 PDT
When this is addressed, we also need to make sure that files are never double-counted, e.g. in the case of an SVN file move, which shows up in the diff as a copy followed by a delete.
Comment 2 Chris Jerdonek 2010-05-25 15:23:04 PDT
*** Bug 39690 has been marked as a duplicate of this bug. ***
Comment 3 Kenichi Ishibashi 2010-08-04 01:03:39 PDT
Hi,

I think the number of files should leave as it stands because it should represent the actual number of files that are checked. So I'd like to fix this issue by counting files that contains only deleted lines separately. I'll send patch soon.

Thanks,
Comment 4 Kenichi Ishibashi 2010-08-04 01:17:03 PDT
Created attachment 63427 [details]
Proposed Patch
Comment 5 Shinichiro Hamaji 2010-08-04 01:31:40 PDT
Comment on attachment 63427 [details]
Proposed Patch

We need to update unittest for them.


WebKitTools/Scripts/webkitpy/style/filereader.py:61
 +          self.processed_file_count = 0
Please update the docstring above.
Comment 6 Kenichi Ishibashi 2010-08-04 02:32:34 PDT
(In reply to comment #5)
Hamaji-san,

Thank you for your quick review.

> (From update of attachment 63427 [details])
> We need to update unittest for them.

I'm sorry for missing tests. I'll add unittests for them.

> WebKitTools/Scripts/webkitpy/style/filereader.py:61
>  +          self.processed_file_count = 0
> Please update the docstring above.

I've noticed that the variable also counts skipped files so that I'll revert the change.
Comment 7 Shinichiro Hamaji 2010-08-04 02:38:36 PDT
> > (From update of attachment 63427 [details] [details])
> > We need to update unittest for them.
> 
> I'm sorry for missing tests. I'll add unittests for them.

Never mind. Thanks for fixing this issue!
Comment 8 Kenichi Ishibashi 2010-08-04 02:52:44 PDT
Created attachment 63431 [details]
Proposed Patch V2
Comment 9 Shinichiro Hamaji 2010-08-04 07:29:57 PDT
Comment on attachment 63431 [details]
Proposed Patch V2

Looks good.
Comment 10 WebKit Commit Bot 2010-08-05 06:42:08 PDT
Comment on attachment 63431 [details]
Proposed Patch V2

Clearing flags on attachment: 63431

Committed r64743: <http://trac.webkit.org/changeset/64743>
Comment 11 WebKit Commit Bot 2010-08-05 06:42:14 PDT
All reviewed patches have been landed.  Closing bug.