Closed Bug 1091270 Opened 5 years ago Closed 5 years ago

Move isURL out of automationutils


(Testing :: General, defect)

Not set


(Not tracked)



(Reporter: jgriffin, Assigned: bitgeeky, Mentored)



(Whiteboard: [good first bug][lang=python])

User Story

Replace the call sites for ( ) with the actual function, then delete it from automationutils.


(1 file, 2 obsolete files)

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

We should remove it from automationutils and replace it with the actual function in those places.
User Story: (updated)
Whiteboard: [good first bug][lang=python]
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.
Assignee: nobody → mozpankaj1994
Attached patch Bug-1091270.patch (obsolete) — Splinter Review
Made the suggested changes, Please let me know if it looks fine.
Attachment #8515390 - Flags: review?(jgriffin)
Attachment #8515390 - Flags: review?(ahalberstadt)
Comment on attachment 8515390 [details] [diff] [review]

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/
@@ +788,5 @@
>    if options.xrePath is None:
>      options.xrePath = os.path.dirname(
> +  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"
Attachment #8515390 - Flags: review?(jgriffin) → review-
Attachment #8515390 - Flags: review?(ahalberstadt)
Attached patch Bug-1091270-v2.patch (obsolete) — Splinter Review
Agreed ! Made the suggested change. Please review.
Attachment #8515390 - Attachment is obsolete: true
Attachment #8516035 - Flags: review?(jgriffin)
Comment on attachment 8516035 [details] [diff] [review]

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.
Attachment #8516035 - Flags: review?(jgriffin) → review-
Sure Thing :-) The last one was just a diff file, here is a properly formatted patch. Please review.
Attachment #8516035 - Attachment is obsolete: true
Attachment #8516624 - Flags: review?(jgriffin)
Comment on attachment 8516624 [details] [diff] [review]

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

lgtm, thanks!  pushed to try:
Attachment #8516624 - Flags: review?(jgriffin) → review+
Closed: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.