WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
94638
Merge TestExpectationSerializer into TestExpectationLine
https://bugs.webkit.org/show_bug.cgi?id=94638
Summary
Merge TestExpectationSerializer into TestExpectationLine
Ryosuke Niwa
Reported
2012-08-21 15:16:37 PDT
Having a separate class to parse and serialize test expectation doesn't seem to buy us much. Just let TestExpectationLine serialize itself.
Attachments
Patch
(35.97 KB, patch)
2012-08-21 15:21 PDT
,
Ryosuke Niwa
dglazkov
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1
2012-08-21 15:21:54 PDT
Created
attachment 159778
[details]
Patch
Dimitri Glazkov (Google)
Comment 2
2012-08-21 17:16:59 PDT
I thought it was a fairly clean separation of responsibilities. TestExpectationLine is already pretty big. Why do you feel like combining is a good idea?
Ryosuke Niwa
Comment 3
2012-08-21 18:17:17 PDT
(In reply to
comment #2
)
> I thought it was a fairly clean separation of responsibilities. TestExpectationLine is already pretty big. Why do you feel like combining is a good idea?
TestExpecations has only 3 functions, two of which are one-liner. The problem is that TestExpectationLine exposes too much of its internal. It basically doesn't have any encapsulation, and that makes the refactoring to add the support for new format harder.
Dimitri Glazkov (Google)
Comment 4
2012-08-22 11:22:26 PDT
Comment on
attachment 159778
[details]
Patch ok.
Ryosuke Niwa
Comment 5
2012-08-22 12:32:08 PDT
Committed
r126334
: <
http://trac.webkit.org/changeset/126334
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug