1387632 - Rename ProfileStorage.jsm to FormAutofillStorage.jsm to make it more clear

Status: RESOLVED FIXED

(Toolkit :: Form Autofill, enhancement, P3)

Description

[Matthew N. [:MattN]]

The "ProfileStorage" name is too generic. It would be much more clear as FormAutofillStorage.jsm to align with the other related JSM files.

Comment 1

[Hossain Al Ikram [:ikram] (QA Contact)]

I am submitting a Patch by renaming the files and the occurrences soon :)

Comment 2

[Hossain Al Ikram [:ikram] (QA Contact)]

Attachment: Renamed-Files.patch

Please review and let me know if I should modify something more :)

Comment 3

[Matthew N. [:MattN]]

Comment on attachment 8894105 [details] [diff] [review]
Renamed-Files.patch

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

Hello,

This is on the right track but the patch is missing the actual move of the JSM file. You can run the following to do the move:
> hg mv browser/extensions/formautofill/ProfileStorage.jsm browser/extensions/formautofill/FormAutofillStorage.jsm

We should also rename the exported symbol from the module to be `formAutofillStorage` instead of `profileStorage` and rename the "ProfileStorage" object too.

Thanks

Comment 4

[Hossain Al Ikram [:ikram] (QA Contact)]

Attachment: Updatedpatch.patch

Patch Updated. Let me know, if this is okay or anything should be modified :)

Comment 5

[Matthew N. [:MattN]]

Comment on attachment 8895267 [details] [diff] [review]
Updatedpatch.patch

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

Hello,

The patch still seems to be missing the JSM that you're renaming: ProfileStorage.jsm/FormAutofillStorage.jsm

Comment 6

[Hossain Al Ikram [:ikram] (QA Contact)]

Attachment: new-patch.patch

Not sure, why this is happening, Sorry for the troubles. Please check this one and let me know about this one.

Comment 7

[Matthew N. [:MattN]]

Comment on attachment 8896026 [details] [diff] [review]
new-patch.patch

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

> diff --git a/browser/extensions/formautofill/ProfileStorage.jsm b/browser/extensions/formautofill/FormAutofillStorage.jsm

I do see the rename this time but you didn't change the EXPORTED_SYMBOLS it seems. Are you using Git or Mercurial?

Comment 8

[Hossain Al Ikram [:ikram] (QA Contact)]

Hey Matthew,

I am using mercurial but I am not sure how do I change the EXPORTED_SYMBOLS?

Comment 9

[Matthew N. [:MattN]]

It's just a variable inside ProfileStorage.jsm

Comment 10

[Hossain Al Ikram [:ikram] (QA Contact)]

Attachment: latest.patch

Please, let me know, if this change is correct or not :)

Comment 11

[Matthew N. [:MattN]]

Comment on attachment 8904703 [details] [diff] [review]
latest.patch

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

Hello,

You don't need to needinfo and set a review flag since the review flag is sufficient to request review.

::: browser/extensions/formautofill/FormAutofillStorage.jsm
@@ +103,4 @@
>  
>  // We expose a singleton from this module. Some tests may import the
>  // constructor via a backstage pass.
> +this.EXPORTED_SYMBOLS = ["FormAutofillStorage"];

Hello, I guess you didn't run the tests[1] since this name needs to match the global to be exported from the file but you didn't rename the variable below.

[1] `mach test browser/extensions/formautofill/`

Comment 12

[George Dan]

Attachment: 1387632.patch

Comment 13

[Matthew N. [:MattN]]

Comment on attachment 8944583 [details] [diff] [review]
1387632.patch

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

Thanks, I was originally thinking we would rename the exported symbol tool but I'll file a new bug for that.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26403995065c
Rename ProfileStorage module to FormAutofillStorage. r=MattN

Comment 15

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/26403995065c