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+
DetailsFormatted DiffDiff
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.