Bug 36093 - webkitpy: Move modules in webkitpy/ into appropriate sub-packages (master bug)
Summary: webkitpy: Move modules in webkitpy/ into appropriate sub-packages (master bug)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 36483 36580
Blocks:
  Show dependency treegraph
 
Reported: 2010-03-13 16:55 PST by Chris Jerdonek
Modified: 2010-03-25 07:47 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Jerdonek 2010-03-13 16:55:20 PST
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?
Comment 1 Adam Barth 2010-03-13 17:21:42 PST
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
Comment 2 Adam Barth 2010-03-13 17:24:08 PST
I know the dependences probably aren't pretty right now, but that's something to aim for at least.
Comment 3 Chris Jerdonek 2010-03-14 03:07:14 PDT
(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/.
Comment 4 Adam Barth 2010-03-14 08:19:03 PDT
> 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.
Comment 5 Chris Jerdonek 2010-03-14 13:19:45 PDT
(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.)
Comment 6 Chris Jerdonek 2010-03-14 13:26:37 PDT
(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.
Comment 7 Adam Barth 2010-03-15 00:00:28 PDT
webkitport => ports is a good idea.  I'm not sure about networktransaction.  I never really liked the name, but I like the class.
Comment 8 Adam Barth 2010-03-15 00:01:26 PDT
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).
Comment 9 Chris Jerdonek 2010-03-15 17:41:55 PDT
(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
Comment 10 Chris Jerdonek 2010-03-23 01:49:03 PDT
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/
Comment 11 Adam Barth 2010-03-24 22:03:11 PDT
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/
Comment 12 Chris Jerdonek 2010-03-24 23:30:56 PDT
Manually committed:

http://trac.webkit.org/changeset/56497

(Moves webkitpy/steps/ to webkitpy/tools/steps.)
Comment 13 Chris Jerdonek 2010-03-25 00:19:32 PDT
Manually committed:

http://trac.webkit.org/changeset/56504

(Moves webkitport.py and committers.py into common/config/.)
Comment 14 Adam Barth 2010-03-25 00:52:53 PDT
webkitpy.common.net in http://trac.webkit.org/changeset/56508
Comment 15 Chris Jerdonek 2010-03-25 01:13:35 PDT
Moved init/versioning.py into python24/versioning.py:

http://trac.webkit.org/changeset/56509
Comment 16 Adam Barth 2010-03-25 01:36:07 PDT
webkitpy.common.checkout in http://trac.webkit.org/changeset/56510
Comment 17 Adam Barth 2010-03-25 01:42:16 PDT
Committed r56512: <http://trac.webkit.org/changeset/56512>
Comment 18 Adam Barth 2010-03-25 01:42:49 PDT
stepsequence in http://trac.webkit.org/changeset/56512
Comment 19 Chris Jerdonek 2010-03-25 01:50:08 PDT
Moved init/ to common/system/:

http://trac.webkit.org/changeset/56514
Comment 20 Adam Barth 2010-03-25 01:50:23 PDT
Committed r56516: <http://trac.webkit.org/changeset/56516>
Comment 21 Adam Barth 2010-03-25 01:50:42 PDT
grammar.py in http://trac.webkit.org/changeset/56516
Comment 22 Adam Barth 2010-03-25 02:01:09 PDT
Committed r56517: <http://trac.webkit.org/changeset/56517>
Comment 23 Adam Barth 2010-03-25 02:05:38 PDT
Committed r56518: <http://trac.webkit.org/changeset/56518>
Comment 24 Adam Barth 2010-03-25 02:11:42 PDT
Committed r56519: <http://trac.webkit.org/changeset/56519>
Comment 25 Adam Barth 2010-03-25 02:17:20 PDT
Committed r56520: <http://trac.webkit.org/changeset/56520>
Comment 26 Chris Jerdonek 2010-03-25 02:32:12 PDT
Divided the unittest files up:

http://trac.webkit.org/changeset/56521
Comment 27 Adam Barth 2010-03-25 02:32:31 PDT
Committed r56522: <http://trac.webkit.org/changeset/56522>
Comment 28 Chris Jerdonek 2010-03-25 07:47:50 PDT
Moved webkit_logging.py to common/system/deprecated_logging.py:

http://trac.webkit.org/changeset/56544