RESOLVED FIXED 124650
Move stray urls into common.config.urls 
https://bugs.webkit.org/show_bug.cgi?id=124650
Summary Move stray urls into common.config.urls
Dániel Bátyai
Reported 2013-11-20 03:16:47 PST
Move stray urls from svn.py and statusserver.py into common.config.urls
Attachments
Proposed patch (4.69 KB, patch)
2013-11-20 03:18 PST, Dániel Bátyai 		no flags
DetailsFormatted DiffDiff
Proposed patch (4.71 KB, patch)
2013-11-20 03:20 PST, Dániel Bátyai 		rniwa: review-
rniwa: commit-queue-
DetailsFormatted DiffDiff
Proposed patch (3.43 KB, patch)
2013-11-20 06:36 PST, Dániel Bátyai 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Dániel Bátyai
Comment 1 2013-11-20 03:18:06 PST
Created attachment 217413 [details] Proposed patch
Dániel Bátyai
Comment 2 2013-11-20 03:20:31 PST
Created attachment 217414 [details] Proposed patch
Ryosuke Niwa
Comment 3 2013-11-20 05:46:24 PST
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.
Dániel Bátyai
Comment 4 2013-11-20 06:24:03 PST
(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?
Ryosuke Niwa
Comment 5 2013-11-20 06:24:45 PST
(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.
Dániel Bátyai
Comment 6 2013-11-20 06:36:11 PST
Created attachment 217423 [details] Proposed patch changed imports
Ryosuke Niwa
Comment 7 2013-11-20 06:44:04 PST
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?
Dániel Bátyai
Comment 8 2013-11-20 06:58:35 PST
(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
WebKit Commit Bot
Comment 9 2013-11-20 07:46:54 PST
Comment on attachment 217423 [details] Proposed patch Clearing flags on attachment: 217423 Committed r159562: <http://trac.webkit.org/changeset/159562>
WebKit Commit Bot
Comment 10 2013-11-20 07:46:56 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.