Closed Bug 865188 Opened 11 years ago Closed 11 years ago

[Storage] Remove "profile" database from OpenSpecialDatabase

Categories

(Toolkit :: Storage, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: Yoric, Assigned: r.deviti)

References

Details

(Keywords: addon-compat, dev-doc-needed, Whiteboard: [good-first-bug][mentor=mak][lang=cpp])

Attachments

(1 file, 1 obsolete file)

      No description provided.
Whiteboard: [good-first-bug][mentor=mak][lang=cpp]
Hi,
I'd like to be assigned to this bug.
Hi, welcome to bugs fixing :)
Assignee: nobody → r.deviti
Status: NEW → ASSIGNED
Attachment #772800 - Flags: review?(mak77)
Comment on attachment 772800 [details] [diff] [review]
All references to "profile" and NS_APP_STORAGE_50_FILE are removed.

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

You should fix the patch format as explained here:
https://developer.mozilla.org/en/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F

Basically, you need the proper annotations for author and commit message.
You can set the author with hg qref -U (for yourself) or hg qref -u "name <mail>" (for someone else). and you can set the commit message with hg qref -m "message" or, if you have setup an editor in the hgrc file you can just use hg qref -e. The message should be like "Bug NNNNNNN - Description of the changes. r=reviewer"
So in this case could be something like "Bug 865188 - Remove Storage support for "profile" special database. r=mak"
The mdn above explains how to setup defaults so that all patches are already annotated with you as the author.

Then you can export the patch with this small trick:
hg log -r "$REV" --template '# HG changeset patch\n# User {author}\n{desc}\n\n' && hg qdiff > /path-to-exported-patch/865188.diff
or you can install the bzexport extension that allows to attach patches from your queue to bugzilla.

There is still something to remove though:

::: profile/dirserviceprovider/src/nsProfileDirServiceProvider.cpp
@@ -228,5 @@
>    }
> -  else if (strcmp(prop, NS_APP_STORAGE_50_FILE) == 0) {
> -    rv = domainDir->Clone(getter_AddRefs(localFile));
> -    if (NS_SUCCEEDED(rv))
> -      rv = localFile->AppendNative(STORAGE_FILE_50_NAME);

you should also remove all of the instances of STORAGE_FILE_50_NAME, since now it would be unused.

::: storage/public/mozIStorageService.idl
@@ +75,5 @@
>    /**
>     * Get a connection to a named special database storage.
>     *
>     * @param aStorageKey a string key identifying the type of storage
> +   * requested.  Valid values include: "memory".

please also remove MOZ_STORAGE_PROFILE_STORAGE_KEY from this file, it looks completely unused already.
Attachment #772800 - Flags: review?(mak77) → feedback+
ehr, I actually meant:
hg log -r tip --template '# HG changeset patch\n# User {author}\n{desc}\n\n' && hg qdiff > /path-to-exported-patch/865188.diff
(In reply to Marco Bonardo [:mak] from comment #4)
> ::: storage/public/mozIStorageService.idl
> @@ +75,5 @@
> >    /**
> >     * Get a connection to a named special database storage.
> >     *
> >     * @param aStorageKey a string key identifying the type of storage
> > +   * requested.  Valid values include: "memory".
> 
> please also remove MOZ_STORAGE_PROFILE_STORAGE_KEY from this file, it looks
> completely unused already.

sorry, this was correct already, I missed it.
Comment on attachment 773550 [details] [diff] [review]
All references to "profile", NS_APP_STORAGE_50_FILE and STORAGE_FILE_50_NAME are removed.

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

I verified on addons code we track the usage of this feature, I could only find an add-on for Thunderbird 3.0, so I think we are in a very good shape to kill this monster.
The patch looks good, based on my research on mxr, there are no internal consumers nor tests for it, you already ran other tests locally, so I'd say we are good to go.

You can set the checkin-needed keyword to ask for someone to push the patch to the tree, after some contributions you may request access to the Try server, and later full access to all of the repository, to push by yourself.

Thank you and congrats for your first contribution.
Attachment #773550 - Flags: review?(mak77) → review+
Setting addon-compat, even if the addons impact is very very limited, it's unlikely someone ever thought to use this hidden feature.
Flags: in-testsuite-
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/aeab42c59423
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.