Bug 124650 – Move stray urls into common.config.urls

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Dániel Bátyai]

Move stray urls from svn.py and statusserver.py into common.config.urls

[Dániel Bátyai]

Created attachment 217413 [details]
Proposed patch

[Dániel Bátyai]

Created attachment 217414 [details]
Proposed patch

[Ryosuke Niwa]

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]

(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]

(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]

Created attachment 217423 [details]
Proposed patch

changed imports

[Ryosuke Niwa]

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]

(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 on attachment 217423 [details]
Proposed patch

Clearing flags on attachment: 217423

Committed r159562: <http://trac.webkit.org/changeset/159562>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.