This is a master bug for discussion purposes, and because these moves will probably be spread over more than one patch. Initial thoughts: * In the end, we probably want webkitpy/ to contain only the following as immediate children: - Package folders - Their associated *_references.py modules. - And perhaps unittests.py. * webkit-patch specific files can go in webkitpy/patch, as was discussed here: https://bugs.webkit.org/show_bug.cgi?id=35499#c9 * Some of the modules used by more than just webkit-patch seem to be-- - diff_parser - scm and its dependencies (changelogs, executive, user, webkit_logging) * We may want to group the scm-related files into an scm package. * We might want to explicitly partition off code that needs to work with Python 2.4 (e.g. into webkitpy/python24). The version-checking code, for example, needs to work with Python 2.4 so we can report a nice error message to Python 2.4 users. By separating it off, it will be easier to ensure that this code does not accidentally import code that requires Python 2.5 and break. Initial questions: (1) Are the webkitpy/commands and webkitpy/steps packages webkit-patch specific? (2) Aside from patch, scm, and any infra-structure related modules that can go in webkitpy/init, do you see any other possibilities for packages that should go immediately under webkitpy? For example, are there any other applications distinct from webkit-patch that rely on the scm-related code in webkitpy?
Here's a candidate layout: webkitpy/ __init__.py init/ layout_tests/ style/ style_references.py thirdparty/ patch/ commands/ commands_references.py steps/ steps_references.py bot/ patchcollection.py patchcollection_unittest.py queueengine.py queueengine_unittest.py stepsequence.py comments.py mock_bugzillatool.py <-- wrong name multicommandtool.py multicommandtool_unittest.py config/ committers.py committers_unittest.py webkitport.py webkitport_unittest.py scm/ changelogs.py changelogs_unittest.py diff_parser.py diff_parser_unittest.py scm.py <-- should become many files scm_unittest.py net/ <-- classes that talk to servers bugzilla.py bugzilla_unittest.py buildbot.py buildbot_unittest.py credentials.py credentials_unittest.py networktransaction.py networktransaction_unittest.py statusserver.py base/ grammar.py grammar_unittest.py executive.py executive_unittest.py outputcapture.py <-- probably should die user.py user_unittest.py webkit_logging.py <-- probably should die webkit_logging_unittest.py unittests.py <-- not sure
I know the dependences probably aren't pretty right now, but that's something to aim for at least.
(In reply to comment #1) > Here's a candidate layout: Here's a suggested revision. For conciseness, I've left out the *_references.py and *_unittest.py files, which should go in as siblings of their corresponding package folders and modules, respectively. webkitpy/ common/ <-- contains modules used by more than one root-level package infra/ autoinstall.py <-- after re-landing the rewrite executive.py logtesting.py logutils.py user.py webkit_logging.py <-- probably should die scm/ changelogs.py diff_parser.py scm.py <-- should become many files layout_tests/ patch/ bot/ patchcollection.py queueengine.py commands/ config/ committers.py webkitport.py net/ <-- classes that talk to servers bugzilla.py buildbot.py credentials.py networktransaction.py statusserver.py steps/ comments.py grammar.py mock_bugzillatool.py <-- wrong name multicommandtool.py outputcapture.py <-- probably should die stepsequence.py python24/ versioning.py style/ thirdparty/ unittests.py <-- not sure I tried moving config/, net/, grammar.py, and outputcapture.py into the patch/ folder because they don't seem to be used by non-patch modules. My initial feeling is that we should try to put modules into the inner-most package in which they are used (thirdparty/ seems to be an exception). This way a module has to "prove" itself before being moved to the shared package at the root level (i.e. common/). This will help us limit the size of common/.
> My initial feeling is that we should try to put modules into the inner-most > package in which they are used (thirdparty/ seems to be an exception). This > way a module has to "prove" itself before being moved to the shared package at > the root level (i.e. common/). This will help us limit the size of common/. That sounds like a good plan. We might want to move config and networktransaction to common now because we have immediate plans to use them in layout_tests.
(In reply to comment #4) > > My initial feeling is that we should try to put modules into the inner-most > > package in which they are used (thirdparty/ seems to be an exception). This > > way a module has to "prove" itself before being moved to the shared package at > > the root level (i.e. common/). This will help us limit the size of common/. > > That sounds like a good plan. We might want to move config and > networktransaction to common now because we have immediate plans to use them in > layout_tests. I assume that would just be config/webkitport.py? Sounds good, though we might want to wait on those until the versioning stuff is decided. It seems like some people may still want to preserve 2.4 for layout_tests, and networktransaction and webkitport both currently require 2.5 (for example, networktransaction imports mechanize, which in turn needs autoinstall). By the way, how do you feel about renaming webkitport.py to ports.py since it will be in a webkitpy config/ directory and contain configurations for multiple ports? And similarly, networktransaction.py -> transaction.py since it will be in a net/ directory? (Just the modules and not the class names within.)
(In reply to comment #5) > And similarly, networktransaction.py -> transaction.py since it will be > in a net/ directory? (Just the modules and not the class names within.) Or perhaps common/infra/net.py (or network.py or networktransaction.py) would be a better location for now.
webkitport => ports is a good idea. I'm not sure about networktransaction. I never really liked the name, but I like the class.
There's also a notion of ports in layout_test. Those need to get merged at some point. There's a patch in review now for layout_test that needs networktransaction (the one about uploading the JSON to AppEngine).
(In reply to comment #8) > There's a patch in review now for layout_test that needs > networktransaction (the one about uploading the JSON to AppEngine). So others know, the patch referred to here is associated with this report: https://bugs.webkit.org/show_bug.cgi?id=36063
Updating the plan for the new hierarchy. (Again leaving out *_references.py and *_unittest.py files.) webkitpy/ common/ <-- contains modules used by more than one root-level package config/ ports.py (was webkitport.py) infra/ autoinstall.py <-- after re-landing the rewrite executive.py logtesting.py logutils.py net.py (was networktransaction.py) user.py webkit_logging.py <-- probably should die scm/ changelogs.py diff_parser.py scm.py <-- should become many files layout_tests/ patch/ bot/ patchcollection.py queueengine.py commands/ config/ committers.py irc/ net/ <-- classes that talk to servers bugzilla.py buildbot.py credentials.py statusserver.py steps/ comments.py grammar.py mock_bugzillatool.py <-- wrong name multicommandtool.py outputcapture.py <-- probably should die stepsequence.py python24/ versioning.py style/ test/ main.py (move code from test-webkitpy) unittests.py thirdparty/
webkitpy/ common/ <-- contains modules used by more than one root-level package config/ ports.py (was webkitport.py) committers.py infra/ autoinstall.py <-- after re-landing the rewrite executive.py logtesting.py logutils.py user.py webkit_logging.py <-- probably should die scm/ changelogs.py diff_parser.py scm.py <-- should become many files net/ <-- classes that talk to servers irc/ bugzilla.py buildbot.py credentials.py networktransaction.py statusserver.py layout_tests/ patch/ bot/ patchcollection.py queueengine.py commands/ steps/ comments.py grammar.py <-- WTF? mock_bugzillatool.py <-- wrong name multicommandtool.py outputcapture.py <-- probably should die stepsequence.py python24/ versioning.py style/ test/ main.py (move code from test-webkitpy) unittests.py thirdparty/
Manually committed: http://trac.webkit.org/changeset/56497 (Moves webkitpy/steps/ to webkitpy/tools/steps.)
Manually committed: http://trac.webkit.org/changeset/56504 (Moves webkitport.py and committers.py into common/config/.)
webkitpy.common.net in http://trac.webkit.org/changeset/56508
Moved init/versioning.py into python24/versioning.py: http://trac.webkit.org/changeset/56509
webkitpy.common.checkout in http://trac.webkit.org/changeset/56510
Committed r56512: <http://trac.webkit.org/changeset/56512>
stepsequence in http://trac.webkit.org/changeset/56512
Moved init/ to common/system/: http://trac.webkit.org/changeset/56514
Committed r56516: <http://trac.webkit.org/changeset/56516>
grammar.py in http://trac.webkit.org/changeset/56516
Committed r56517: <http://trac.webkit.org/changeset/56517>
Committed r56518: <http://trac.webkit.org/changeset/56518>
Committed r56519: <http://trac.webkit.org/changeset/56519>
Committed r56520: <http://trac.webkit.org/changeset/56520>
Divided the unittest files up: http://trac.webkit.org/changeset/56521
Committed r56522: <http://trac.webkit.org/changeset/56522>
Moved webkit_logging.py to common/system/deprecated_logging.py: http://trac.webkit.org/changeset/56544