Bug 94545

Summary: Cleanup TestExpectationParser.parse
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: New BugsAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, tony, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 89161    
Attachments:
Description Flags
Cleanup none

Ryosuke Niwa
Reported 2012-08-20 16:32:49 PDT
Cleanup TestExpectationParser.parse
Attachments
Cleanup (11.22 KB, patch)
2012-08-20 16:39 PDT, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2012-08-20 16:39:14 PDT
Created attachment 159560 [details] Cleanup
Dirk Pranke
Comment 2 2012-08-20 22:08:02 PDT
Comment on attachment 159560 [details] Cleanup So, the change looks more or less correct, but it's not obvious to me that it's better. Can you update the changelog to say why you think things are better (or just update the bug)? Maybe it'll make subsequent refactoring easier or something?
Ryosuke Niwa
Comment 3 2012-08-20 22:11:06 PDT
(In reply to comment #2) > (From update of attachment 159560 [details]) > So, the change looks more or less correct, but it's not obvious to me that it's better. Can you update the changelog to say why you think things are better (or just update the bug)? Maybe it'll make subsequent refactoring easier or something? I'm trying to reduce the number of functions to make the subsequent refactoring easier. Also, the check for having both timeout and slow was done in _parse_line instead of _parse_modifiers where other modifier errors were generated.
Dirk Pranke
Comment 4 2012-08-20 22:12:37 PDT
ok.
WebKit Review Bot
Comment 5 2012-08-20 22:57:00 PDT
Comment on attachment 159560 [details] Cleanup Clearing flags on attachment: 159560 Committed r126135: <http://trac.webkit.org/changeset/126135>
WebKit Review Bot
Comment 6 2012-08-20 22:57:03 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.