Replace custom function LOG() with do_print

RESOLVED FIXED in Firefox 40

Status

()

defect
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: mak, Assigned: rshamley, Mentored)

Tracking

unspecified
mozilla40
Points:
---

Firefox Tracking Flags

(firefox40 fixed)

Details

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

Attachments

(1 attachment)

Reporter

Description

4 years ago
we have a custom logger in toolkit/components/places/tests/head_common.js#61 called LOG() that is used in some tests:
mxr.mozilla.org/mozilla-central/search?string=LOG(&case=on&find=places

we should replace the LOG() calls with do_print, and remove the custom logger.
Assignee

Comment 1

4 years ago
Hi Marco, I'm interested in fixing this bug. I have Firefox built and found the references in the code from the link to mxr. How should I proceed to fix the problem?
Flags: needinfo?(mak77)
Reporter

Comment 2

4 years ago
these are the calls to replace, 
http://mxr.mozilla.org/mozilla-central/search?string=LOG(&case=on&find=places

they should be changed from LOG( to do_print(
Flags: needinfo?(mak77)
Assignee

Comment 3

4 years ago
I'm new to Mercurial and patches in general. I believe I've done this correctly. I'm unsure of how exactly to proceed if this patch is deemed acceptable however.
Attachment #8588067 - Flags: review?(mak77)
Reporter

Updated

4 years ago
Assignee: nobody → rshamley
Status: NEW → ASSIGNED
Reporter

Comment 4

4 years ago
Comment on attachment 8588067 [details] [diff] [review]
I replaced the LOG() calls with do_print() and removed the LOG() function

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

There is a small typo in the commit message "removed" should be "remove"

also, you can use nicknames for reviewers, so in this case r=mak would suffice.

::: toolkit/components/places/tests/bookmarks/test_417228-other-roots.js
@@ +28,5 @@
>      do_check_eq(rootNode.childCount, 4);
>  
>      // create a test root
>      this._folderTitle = "test folder";
> +    this._folderId =

your editor is setup to remove all trailing spaces in the files you touch.
While it's not a big deal here since you are only touching tests, it usually is.
The best setting is for it to remove trailing spaces from the LINES you touch.

::: toolkit/components/places/tests/queries/test_results-as-tag-contents-query.js
@@ +96,5 @@
>                    title: "mozadded",
>                    isBookmark: true,
>                    isTag: true,
>                    tagArray: ["moz", "bugzilla"] };
> +  do_print("Adding item to query")

missing semicolon here. it was already broken but please fix it since you're touching this code.

::: toolkit/components/places/tests/queries/test_searchterms-domain.js
@@ +87,5 @@
>    compareArrayToResult(testData, root);
>  
>    // If that passes, check liveupdate
>    // Add to the query set
> +  do_print("Adding item to query")

missing semicolon (again, not your fault but please fix it)

::: toolkit/components/places/tests/queries/test_searchterms-uri.js
@@ +83,5 @@
>     compareArrayToResult(testData, root);
>  
>     // If that passes, check liveupdate
>     // Add to the query set
> +   do_print("Adding item to query")

another one
Attachment #8588067 - Flags: review?(mak77) → feedback+
Reporter

Comment 6

4 years ago
we should be fine to checkin once we get the updated patch.
Reporter

Comment 7

4 years ago
I'll take care of landing this.
Flags: needinfo?(mak77)
Reporter

Updated

4 years ago
Flags: needinfo?(mak77)
Attachment #8588067 - Flags: feedback+ → review+
https://hg.mozilla.org/mozilla-central/rev/fe67ac8df399
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.