Move stray urls from svn.py and statusserver.py into common.config.urls
Created attachment 217413 [details] Proposed patch
Created attachment 217414 [details] Proposed patch
Comment on attachment 217414 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=217414&action=review > Tools/Scripts/webkitpy/common/checkout/scm/svn.py:38 > +import webkitpy.common.config.urls as config_urls We don't use this style of import in webkitpy. Please import svn_server_host and svn_server_realm separately using "from ~ import ~" below. > Tools/Scripts/webkitpy/common/net/statusserver.py:36 > +import webkitpy.common.config.urls as config_urls Ditto. > Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py:-40 > +import webkitpy.common.config.urls as config_urls > > from webkitpy.common import find_files > from webkitpy.common.checkout.scm.detection import SCMDetector > -from webkitpy.common.config.urls import view_source_url This is just a bad change.
(In reply to comment #3) > We don't use this style of import in webkitpy. > Please import svn_server_host and svn_server_realm separately using "from ~ import ~" below. Actually, I've found four other locations where it's used like this, should I change those as well then?
(In reply to comment #4) > (In reply to comment #3) > > We don't use this style of import in webkitpy. > > Please import svn_server_host and svn_server_realm separately using "from ~ import ~" below. > > Actually, I've found four other locations where it's used like this, should I change those as well then? We can fix that later.
Created attachment 217423 [details] Proposed patch changed imports
Comment on attachment 217423 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=217423&action=review > Tools/Scripts/webkitpy/common/checkout/scm/svn.py:51 > + svn_server_host = svn_server_host > + svn_server_realm = svn_server_realm Why do we need these variables? We could just use these values directly in where they're used, right? Also, did you run test-webkitpy inside a subversion checkout with —all option?
(In reply to comment #7) > Why do we need these variables? We could just use these values directly in where they're used, right? Other classes need them, like Git and SVN, and in some tests they are set to different values than what is imported from urls.py, so I had to keep them. > Also, did you run test-webkitpy inside a subversion checkout with —all option? Yes, I did, it ran flawlessly
Comment on attachment 217423 [details] Proposed patch Clearing flags on attachment: 217423 Committed r159562: <http://trac.webkit.org/changeset/159562>
All reviewed patches have been landed. Closing bug.