| Summary: | [bzt] Kill LandingSequence | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | eric, webkit.review.bot | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | All | ||||||
| OS: | All | ||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 32406, 32467 | ||||||
| Attachments: |
|
||||||
|
Description
Adam Barth
2009-12-11 23:45:46 PST
Created attachment 44728 [details]
Patch
style-queue ran check-webkit-style on attachment 44728 [details] without any errors.
Comment on attachment 44728 [details]
Patch
Now we'll need to test the input/output state of each command. Seems this would be easier to test if they were in fact exactly that. input = {} and output = {}. Do we ever need output to persist across comamnds? e.g. that command1 produces output that command3 uses, but command2 ignores? Donno.
Spacing?
196 state = {"patch": patch}
I would have put spaces after { and before }... but maybe the PEP says something here? Also I would have put a space before :
Seems like Command should expose a run_and_handle_errors method, which calls through to the _sequence method.
Looks OK though. Would be good to figure out what the official spacing is supposed to be, since your spacing here is inconsistent with at least the code that I have written in modules/*.py
Ok. I'll fix the spacing thing at the end. It's going to conflict otherwise. We discussed the input/output thing in person. That's to make the steps more loosely coupled. Comment on attachment 44728 [details] Patch Clearing flags on attachment: 44728 Committed r52129: <http://trac.webkit.org/changeset/52129> All reviewed patches have been landed. Closing bug. |