36093 2010-03-13 16:55:20 -0800 webkitpy: Move modules in webkitpy/ into appropriate sub-packages (master bug) 2010-03-25 07:47:50 -0700 1 1 1 Unclassified WebKit Tools / Tests 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 36483 36580 1 cjerdonek webkit-unassigned abarth cjerdonek eric hamaji levin oldest_to_newest 199430 0 cjerdonek 2010-03-13 16:55:20 -0800 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? 199432 1 abarth 2010-03-13 17:21:42 -0800 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 199433 2 abarth 2010-03-13 17:24:08 -0800 I know the dependences probably aren't pretty right now, but that's something to aim for at least. 199457 3 cjerdonek 2010-03-14 03:07:14 -0700 (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/. 199467 4 abarth 2010-03-14 08:19:03 -0700 > 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. 199484 5 cjerdonek 2010-03-14 13:19:45 -0700 (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.) 199486 6 cjerdonek 2010-03-14 13:26:37 -0700 (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. 199552 7 abarth 2010-03-15 00:00:28 -0700 webkitport => ports is a good idea. I'm not sure about networktransaction. I never really liked the name, but I like the class. 199554 8 abarth 2010-03-15 00:01:26 -0700 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). 199933 9 cjerdonek 2010-03-15 17:41:55 -0700 (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 202969 10 cjerdonek 2010-03-23 01:49:03 -0700 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/ 203788 11 abarth 2010-03-24 22:03:11 -0700 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/ 203826 12 cjerdonek 2010-03-24 23:30:56 -0700 Manually committed: http://trac.webkit.org/changeset/56497 (Moves webkitpy/steps/ to webkitpy/tools/steps.) 203860 13 cjerdonek 2010-03-25 00:19:32 -0700 Manually committed: http://trac.webkit.org/changeset/56504 (Moves webkitport.py and committers.py into common/config/.) 203885 14 abarth 2010-03-25 00:52:53 -0700 webkitpy.common.net in http://trac.webkit.org/changeset/56508 203895 15 cjerdonek 2010-03-25 01:13:35 -0700 Moved init/versioning.py into python24/versioning.py: http://trac.webkit.org/changeset/56509 203904 16 abarth 2010-03-25 01:36:07 -0700 webkitpy.common.checkout in http://trac.webkit.org/changeset/56510 203911 17 abarth 2010-03-25 01:42:16 -0700 Committed r56512: <http://trac.webkit.org/changeset/56512> 203912 18 abarth 2010-03-25 01:42:49 -0700 stepsequence in http://trac.webkit.org/changeset/56512 203924 19 cjerdonek 2010-03-25 01:50:08 -0700 Moved init/ to common/system/: http://trac.webkit.org/changeset/56514 203925 20 abarth 2010-03-25 01:50:23 -0700 Committed r56516: <http://trac.webkit.org/changeset/56516> 203928 21 abarth 2010-03-25 01:50:42 -0700 grammar.py in http://trac.webkit.org/changeset/56516 203940 22 abarth 2010-03-25 02:01:09 -0700 Committed r56517: <http://trac.webkit.org/changeset/56517> 203942 23 abarth 2010-03-25 02:05:38 -0700 Committed r56518: <http://trac.webkit.org/changeset/56518> 203943 24 abarth 2010-03-25 02:11:42 -0700 Committed r56519: <http://trac.webkit.org/changeset/56519> 203947 25 abarth 2010-03-25 02:17:20 -0700 Committed r56520: <http://trac.webkit.org/changeset/56520> 203951 26 cjerdonek 2010-03-25 02:32:12 -0700 Divided the unittest files up: http://trac.webkit.org/changeset/56521 203952 27 abarth 2010-03-25 02:32:31 -0700 Committed r56522: <http://trac.webkit.org/changeset/56522> 204059 28 cjerdonek 2010-03-25 07:47:50 -0700 Moved webkit_logging.py to common/system/deprecated_logging.py: http://trac.webkit.org/changeset/56544