|Summary:||check-webkit-style returns non-zero when patch is entirely minus lines|
|Product:||WebKit||Reporter:||Eric Seidel (no email) <eric>|
|Component:||Tools / Tests||Assignee:||Nobody <webkit-unassigned>|
|Severity:||Normal||CC:||bashi, cjerdonek, commit-queue, hamaji, hayato, ojan|
|Version:||528+ (Nightly build)|
|OS:||OS X 10.5|
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.