Closed Bug 1509464 Opened 2 years ago Closed 1 year ago

Remove PlacesUtils.setAnnotationsForItem

Categories

(Toolkit :: Places, enhancement, P2)

Product:
Toolkit
Component:
Places
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla68
Tracking Flags:
Tracking Status
firefox68 --- fixed

People

(Reporter: standard8, Assigned: mihir17166, Mentored)

References

(Blocks 1 open bug)

Details

(Whiteboard: [fxsearch])

Attachments

(1 file, 2 obsolete files)

Bug 1509464 - Remove PlacesUtils.setAnnotationsForItem. r?Standard8
47 bytes, text/x-phabricator-request
Details | Review 
Following bug 1489409, we should be close enough to removing PlacesUtils.setAnnotationsForItem completely.

We no longer need to save annotations for transactions (in createItemsFromBookmarksTree), and we don't need to handle transactions when we're inserting a tree (via handleBookmarkItemSpecialData).

Alex is going to look at this.

Assignee: nobody → jrkong.hfd
Mentor: standard8
Status: NEW → ASSIGNED

Note: The relevant tests to run for these are in toolkit/components/places and browser/components/places.

Doing these should be enough (unless you spot other instances outside these directories in the code base, then just fix those & run the appropriate test):

./mach xpcshell-test toolkit/components/places
./mach mochitest toolkit/components/places
./mach xpcshell-test browser/components/places
./mach mochitest browser/components/places

test_utils_setAnnotationsForItem.js can be removed completely.

need to remove all of these?
https://searchfox.org/mozilla-central/search?q=PlacesUtils.setAnnotationsForItem&path=

(In reply to Mark Banner (:standard8) (afk until Friday) from comment #2)

Note: The relevant tests to run for these are in toolkit/components/places and browser/components/places.

Doing these should be enough (unless you spot other instances outside these directories in the code base, then just fix those & run the appropriate test):

./mach xpcshell-test toolkit/components/places
./mach mochitest toolkit/components/places
./mach xpcshell-test browser/components/places
./mach mochitest browser/components/places

test_utils_setAnnotationsForItem.js can be removed completely.

Flags: needinfo?(standard8)

(In reply to Manish [:manishkk][Less Active until 24 Feb] from comment #3)

need to remove all of these?
https://searchfox.org/mozilla-central/search?q=PlacesUtils.setAnnotationsForItem&path=

Yes that is the case, but please leave this bug for Alex as per the assignee field. He was stalled for various reasons but is hoping to do this soon.

Flags: needinfo?(standard8)

Passing over to Mihir, as Alex isn't able to work on this for a while.

Assignee: jrkong.hfd → mihir17166

Depends on D24260

Attachment #9052400 - Attachment is obsolete: true
Attachment #9052389 - Attachment is obsolete: true
Pushed by mbanner@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/03162ab7285e
Remove PlacesUtils.setAnnotationsForItem. r=Standard8

https://hg.mozilla.org/mozilla-central/rev/03162ab7285e

Status: ASSIGNED → RESOLVED
Closed: 1 year ago
status-firefox68: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
You need to log in before you can comment on or make changes to this bug.