615244 - the 'self' module should be high-level

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect, P1)

Description

[Dietrich Ayala (:dietrich)]

or at least portions of it.

we require usage of it in order to load content from the data directory.

since this usage is required for consuming high-level apis, it means it is not a low-level api.

Comment 1

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

From IRC:
[17:48]	<warner>	although actually "self" is now a magic module, and self.js exports a "self-maker function" instead of .data and .id and stuff
	(which would probably be less confusing if we were to rename self.js to self-maker.js, actually)
	<warner>	which actually means that you'll get the same code no matter where you import it from, because "self" doesn't strictly live in any particular package anymore
	<KWierso>	so the shell in addon-kit _would_ work?
	<warner>	KWierso: it wouldn't even be a shell. The self-is-magic loader code always pulls self.js from one particular place, irrespective of what module is importing it. Really all we need to do is move self.md into the other package, so it looks "high-level": the .js code won't even notice.

Comment 2

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

(In reply to comment #1)
> Really all we need to do is move self.md into the other
> package, so it looks "high-level": the .js code won't even notice.

This doesn't seem to work, the module's documentation doesn't seem to show up unless the .md file in /doc matches a .js file in /lib .

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I guess github tweaks extension is broken since Bugzilla 4? I've never used it to do a pull request, and it failed.

Anyway, here's a pull request that might be acceptable:
https://github.com/mozilla/addon-sdk/pull/153#
It adds a symlink from addon-kit/docs/self.md to api-utils/docs/self.md so that we don't have to worry about keeping two copies of the documentation around.

It also adds an empty self.js file to addon-kit/lib/ because the documentation doesn't list the .md files without a matching .js file. (Should this also just be a symlink?)



This worked for me on Linux. cfx docs shows self in both addon-kit and api-utils sections, with changes to the actual copy in api-utils appearing in both places.
Doing require("self") in a test addon seems to always use the api-utils version, and self.data.url("somefile") makes a url that goes to my test addon's data folder.

No clue how the symlink would work on Windows.

Comment 4

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Hi Wes, thanks a lot for picking this up and for a pull request. Unfortunately, though we can not accept this change as symlinks are not going to work on windows :( 

On the other hand we have bug-654983 on it's way that will allow fixing both this and bug-615921 in following way:

1. Moving docs to the "addon-kit".
2. Creating high level module that proxies to the a low level equivalent. In this case it's going to look like: `module.exports = require("api-utils/self");`.

Can you please modify your pull request accordingly and assign review to me ? 
I'll make sure to review that and land, as soon as we have bug-654983.

Also even though bug-654983 is not ready to land yet, it already contains bits that makes `require("package/module")` work, so you can use that to verify updated patch.

Comment 5

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

(In reply to comment #3)
> I guess github tweaks extension is broken since Bugzilla 4? I've never used
> it to do a pull request, and it failed.
> 

Actually github tweaks extension works just fine with Bugzilla 4 and I'm using it since dietrich updated it to fix some bugs: http://autonome.wordpress.com/2011/03/29/githubbugzilla-add-on-updated/

If you are not able to use it just mention `@gozala` in your pull request so I'm notified and I'll make sure to link it as a review request here.

Thanks!

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

I'm actually away from my computer until sometime next week. Won't be able to work on this until at least Monday.

Someone else is more than welcome to hijack this bug from me in the meantime. :)

Comment 7

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/173

Pointer to Github pull-request

Comment 8

[Will Bamberg [:wbamberg]]

Comment on attachment 534712 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/173

The new location looks good; but all the links to 'api-utils/docs/self' would need to be updated before this patch can land.

Comment 9

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Good catch, I have not realized that! I'll update patch

Comment 10

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Will I'm not able to find such references via grepping all docs. Are you sure we have such links ?

Comment 11

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

Ignore my last comment, I have updated a pull request.

Comment 12

[Will Bamberg [:wbamberg]]

Comment on attachment 534712 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/173

Looks good to me!

Comment 13

[Irakli Gozalishvili [:irakli] [:gozala] [@gozala]]

https://github.com/mozilla/addon-sdk/commit/99f71ec82f842eef689c97406befad9797fcd4c0

Comment 14

[Will Bamberg [:wbamberg]]

Attachment: I missed a couple of references to self.

Comment 15

[Brian Warner [:warner :bwarner]]

Comment on attachment 535078 [details] [diff] [review]
I missed a couple of references to self.

looks good!

Comment 16

[Will Bamberg [:wbamberg]]

Fixed by https://github.com/mozilla/addon-sdk/commit/e3a7dc6ab6493d078b04bf900950f8e5f6a7747b.

Thanks for the review.