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+
Ryosuke Niwa
Comment 1 2012-08-21 15:21:54 PDT
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
Note You need to log in before you can comment on or make changes to this bug.