| Summary: | Move pywebsocket into webkitpy/thirdparty | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Jerdonek <cjerdonek> | ||||||
| Component: | Tools / Tests | Assignee: | Chris Jerdonek <cjerdonek> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, cjerdonek, eric, hamaji, levin, ukai | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | All | ||||||||
| OS: | All | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Jerdonek
2010-02-20 14:51:39 PST
Created attachment 49135 [details]
Proposed patch
As before, this patch does not show the files re-added (since svn-create-patch cannot handle directory moves). cq- for the same reason.
I don't remember the resolution on whether we have the canonical copy of this library or whether http://code.google.com/p/pywebsocket/ is. Also, it seems strange to put it in webkitpy because it's a standalone tool, independent of our python infrastructure... I'm not necessarily opposed, but we should solicit feedback from others. (In reply to comment #2) > I don't remember the resolution on whether we have the canonical copy of this > library or whether http://code.google.com/p/pywebsocket/ is. If we have the canonical copy, we should consider moving it into webkitpy. We do not have the cannoical copy. I believe this is a separate code.google.com project. (In reply to comment #2) > I don't remember the resolution on whether we have the canonical copy of this > library or whether http://code.google.com/p/pywebsocket/ is. > > Also, it seems strange to put it in webkitpy because it's a standalone tool, > independent of our python infrastructure... I'm not necessarily opposed, but > we should solicit feedback from others. Things like check-webkit-style and the new Python layout test package are also stand-alone tools that we are putting in webkitpy (in addition to exposing more publicly via a wrapper script in the Scripts/ directory). They are not really meant to be referenced by other modules in webkitpy. In other words, webkitpy in my mind is not so much a library of reusable Python code as it is a containing directory for all of our Python code. A nice feature of this is that we know where to go to find our Python code. And with webkitpy/thirdparty, we know where to go to find our third-party Python code. If we want to go another route with webkitpy, we may want to discuss. Another route, for example, could be to put in webkitpy only the code that is meant to be reused across our tools (to make it a package in the truer sense of the word), and to make all of our stand-alone Python tools siblings of that (or at least outside of webkitpy). This would require a bit of extra work, thought, and refactoring, etc. But for the third party code, it shouldn't really require extra work since those are already stand-alone by nature, and so will not have dependencies on other code inside webkitpy. That sounds OK... but that would require moving more of the webkit-patch stuff into its own module. Also the run-webkit-tests stuff uses some of the webkit-patch module now. I expect that other scripts may as well. I don't know that it makes sense to draw lines in the sand like that. (In reply to comment #6) > That sounds OK... but that would require moving more of the webkit-patch stuff > into its own module. Which sounds okay -- (1) having webkitpy contain all of our Python code or (2) having webkitpy contain only code meant to be reused? > Also the run-webkit-tests stuff uses some of the webkit-patch module now. I > expect that other scripts may as well. I don't know that it makes sense to > draw lines in the sand like that. I'm okay staying with (1) for now since (2) is a large-ish change and should probably be thought through. I made the suggestion of (2) primarily in response to Adam's comment 2. (2) sounds like it may be a better organization in the long-run (in some ways a natural extension of what we're already moving towards). But I haven't really thought through the disadvantages yet. For example, it might be somewhat harder to track what code depends on webkitpy if we're not careful about how we organize the code dependent on it. Comment on attachment 49135 [details] Proposed patch I like everything here (I've very much disliked having this external code look like it could be webkit code but have different conventions about things). I have one request, since this is a large module, I would love for it to have a README.webkit (just inside of its directory). Ideally, it should explain how to get the code, the current revision obtained, and any local modifications. (How to get the code is in this bug, the current revision obtained is unknown but could be filled in the next time this module is updated, there are no local modifications). Here's a possible example (though it says more than what I mentioned): http://src.chromium.org/viewvc/chrome/trunk/src/third_party/lzma_sdk/README.chromium?view=markup Created attachment 49531 [details]
Proposed patch 2
Thanks, David! And good suggestion. I've stubbed out a README.webkit. Over time we can add README.webkit files for our other third-party code. Thanks.
Committed (via "svn ci"): http://trac.webkit.org/changeset/55286 |