[webkit-reviews] review denied: [Bug 32406] [bzt] Convert rollout to StepSequence : [Attachment 44729] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Dec 14 16:10:29 PST 2009
Eric Seidel <eric at webkit.org> has denied Adam Barth <abarth at webkit.org>'s
request for review:
Bug 32406: [bzt] Convert rollout to StepSequence
https://bugs.webkit.org/show_bug.cgi?id=32406
Attachment 44729: Patch
https://bugs.webkit.org/attachment.cgi?id=44729&action=review
------- Additional Comments from Eric Seidel <eric at webkit.org>
test_complete_rollout is clearly wrong.
We should just do:
from modules.buildsteps import *
If we want to restrict what * means, we can define a __ALL__ in buildsteps.py:
"The public names defined by a module are determined by checking the module’s
namespace for a variable named __all__; if defined, it must be a sequence of
strings which are names defined or imported by that module. The names given in
__all__ are all considered public and are required to exist. If __all__ is not
defined, the set of public names includes all names found in the module’s
namespace which do not begin with an underscore character ('_'). __all__ should
contain the entire public API. It is intended to avoid accidentally exporting
items that are not part of the API (such as library modules which were imported
and used within the module)."
http://docs.python.org/reference/simple_stmts.html#import
This is probably wrong, yes:
355 self._tool.bugs.reopen_bug(bug_id, state["commit_text"])
it should be the bug link, whatever that is. Do we need a step to convert from
commit text output to revisions? Or maybe we have a class to do that
already...
This feels like this should be done by some StepSequence member:
336 collected_options = cls._collect_options_from_steps(cls.substeps)
I remember you were explaining to me at some point why that was not possible?
Otherwise looks OK. r- for the broken rollout test (which perhaps we should
just remove?)
More information about the webkit-reviews
mailing list