1091270 - Move isURL out of automationutils

Status: RESOLVED FIXED

(Testing :: General, defect)

Description

[Jonathan Griffin (:jgriffin)]

isURL is a single-line function used in only three places:

http://dxr.mozilla.org/mozilla-central/search?q=isURL+ext%3Apy&case=true

We should remove it from automationutils and replace it with the actual function in those places.

Comment 1

[Pankaj Malhotra (:bitgeeky)]

Hi, I contribute to the Web QA Automation projects and now willing to learn Firefox Automation too. I will like to work on this one.

Comment 2

[Pankaj Malhotra (:bitgeeky)]

Attachment: Bug-1091270.patch

Made the suggested changes, Please let me know if it looks fine.

Comment 3

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8515390 [details] [diff] [review]
Bug-1091270.patch

Review of attachment 8515390 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good!  Can you just make the adjustment below in all places the method was copied to?

::: layout/tools/reftest/runreftest.py
@@ +788,5 @@
>  
>    if options.xrePath is None:
>      options.xrePath = os.path.dirname(options.app)
>  
> +  if options.symbolsPath and not len(urlparse(options.symbolsPath).scheme) >= 2:

I think it's a bit more readable to use "len() < 2", rather than "not len() >= 2"

Comment 4

[Pankaj Malhotra (:bitgeeky)]

Attachment: Bug-1091270-v2.patch

Agreed ! Made the suggested change. Please review.

Comment 5

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8516035 [details] [diff] [review]
Bug-1091270-v2.patch

Review of attachment 8516035 [details] [diff] [review]:
-----------------------------------------------------------------

This looks great, but it needs to be rebased; it doesn't apply cleanly to mozilla-central any longer.  Can you attach a rebased version, please?  Thanks.

Comment 6

[Pankaj Malhotra (:bitgeeky)]

Attachment: Bug-1091270-v3.patch

Sure Thing :-) The last one was just a diff file, here is a properly formatted patch. Please review.

Comment 7

[Jonathan Griffin (:jgriffin)]

Comment on attachment 8516624 [details] [diff] [review]
Bug-1091270-v3.patch

Review of attachment 8516624 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm, thanks!  pushed to try: https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=69598c0781bc

Comment 8

[Jonathan Griffin (:jgriffin)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/585a22940ba7

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/585a22940ba7