915376 - (native-jetpack) [tracking] Native Jetpack

Status: RESOLVED INCOMPLETE

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

Comment 1

[Erik Vold [:erikvold] (please needinfo? me)]

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

Pointer to Github pull-request

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

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

Feedback?

Comment 3

[Erik Vold [:erikvold] (please needinfo? me)]

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

adsfdf

Comment 4

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

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

Putting r- just to indicate that more work needs to be done.

Comment 5

[Jeff Griffiths (:canuckistani) (:⚡︎)]

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

r- to follow with Irakli's comment, but this is a really great start!

Comment 6

[Dave Townsend [:mossop]]

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

Everyone else was so negative with feedback-, have a +!

Seriously though this is coming along, just waiting on another round after comments.

Comment 7

[Erik Vold [:erikvold] (please needinfo? me)]

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

I've updated the pull request, ready for more feedback.  I also wrote responses to a few questions asked in the PR and asked a few of my own.

For the most part I want to leave tests for a later phase of the project, since I don't have good answers for the questions Irakli brought up, and I'd like to talk to him more about this now that I've looked in to it.

Comment 8

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Just to capture what we were talking about at lunch, I feel that the prioritization should be to provide in-browser development and packaging first, if we can continue to develop the SDK using the old tools, eg mainly running tests. When we go to web developers interested in extending the developer tools i want to be able to provide them the best possible experience and the ability to load a Jetpack from a directory without packaging is key to this.

Comment 9

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

Erik, I think it's a good description of the high level goals although I would like to have some idea
about implementation plan, donno if that belongs in a JEP or if it would makes sense for us to just go
through it, maybe it's best for us to make a meeting to discuss implementation details and agree what
can go into JEP.

Another thing I noticed that there is only plan for Phase 1, I think it would be useful to also specify
next phases. Very likely we end up changing details of future phases, but still IMO it's useful to have
a bigger picture of how we plan to get there.

Comment 10

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

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

Erik I would like you to reflect few things we've talked about
on the meeting:

1. What are the changes made to loader to make it work. More specifically
   do we just deprecate all dependencies and there for don't need manifest or
   do we use that stop gap solution of `aliases` hack we talked. Or do we
   implement full runtime / install time npm support. This is important so
   we can start communicating our decisions to our users.

2. There are interesting discussion around localization in the bug you've
   created. I think JEP should reflect how are we going to go about it. Most
   importantly how are we migrating existing add-on's to new approach and
   in at what state.

3. Since we don't quite get rid of cfx what are the changes we're making to it.
   how does this affects internal add-on layout, our current strip unused
   modules feature etc... I think we'll be dropping a lot of features so it's
   important to know which ones and communicate to our users with alternatives
   or at least give them time to cook their own solutions.

4. How does this changes affect test infrastructure if at all. I suspect it does
   as there is a special code path in cfx with tons of workarounds when running
   our tests.

This may sound like a tons of questions, but I think it's really useful to have
this bigger picture, otherwise we're going blind.

Thanks

Comment 11

[Dave Townsend [:mossop]]

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

I'm not sure there is much more to be said about this document. Splitting it into bugs and hammering on details there is better I think

Comment 12

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #10)
> Comment on attachment 804776 [details]
> Pointer to Github pull request:
> https://github.com/mozilla/addon-sdk/pull/1241
> 
> Erik I would like you to reflect few things we've talked about
> on the meeting:
> 
> 1. What are the changes made to loader to make it work. More specifically
>    do we just deprecate all dependencies and there for don't need manifest or
>    do we use that stop gap solution of `aliases` hack we talked. Or do we
>    implement full runtime / install time npm support. This is important so
>    we can start communicating our decisions to our users.

Hey Irakli, I agree this is important, this is a decision for you or Dave though.  I suppose it would depend on how much time things take too.  I was planning ignoring the dependencies for now while I work on other things, which would be your first option mentioned, and we can layer the 2nd and 3rd parts on later, I think we will need the alias piece, and obviously we need the npm support at some point.

There is another option of keeping support for the 3rd party packages when `cfx xpi` or `cfx run` is used, but I gathered you want to deprecated this for the npm method?

Comment 13

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Irakli Gozalishvili [:irakli] [:gozala] [@gozala] from comment #10)
> Comment on attachment 804776 [details]
> Pointer to Github pull request:
> https://github.com/mozilla/addon-sdk/pull/1241
> 
> Erik I would like you to reflect few things we've talked about
> on the meeting:
>
> 2. There are interesting discussion around localization in the bug you've
>    created. I think JEP should reflect how are we going to go about it. Most
>    importantly how are we migrating existing add-on's to new approach and
>    in at what state.

Yes I agree, after bug 935290 comment 3 I realize this will be trickier than I thought.

> 3. Since we don't quite get rid of cfx what are the changes we're making to
> it.
>    how does this affects internal add-on layout, our current strip unused
>    modules feature etc... I think we'll be dropping a lot of features so it's
>    important to know which ones and communicate to our users with
> alternatives
>    or at least give them time to cook their own solutions.

Which features do you think we'll be dropping?  I wasn't aware that we currently stripped unused modules, that makes sense.  Was it considered a feature though?

We might be dropping the third party dependencies but that isn't due to this bug it seems to me, and we're only thinking about the decision to make that change at the same time, I don't think that is finalized yet.

I agree that if this change is going to be packaged with some other updates then we should keep track of that major project someplace like bugzilla.  Then someone can blog about it or document it some other way.

> 4. How does this changes affect test infrastructure if at all. I suspect it
> does
>    as there is a special code path in cfx with tons of workarounds when
> running
>    our tests.

Hmm I thought I wrote this down already.. I will add it to the JEP!

Comment 14

[Erik Vold [:erikvold] (please needinfo? me)]

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

Made the following changes, ready for another review:

* describes cuddlefish replacement plan
* describing test harness plans
* adding bug links
* extended phase 2 work description to describe an add-on as a replacement for Flightdeck
* package.json issues mentioned
* more that I forgot already.

Comment 15

[Jeff Griffiths (:canuckistani) (:⚡︎)]

Not sure how much more positive feedback I can give on this. Erik - great job!

Comment 16

[Dave Townsend [:mossop]]

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

I didn't dive into Phase 2 but the earlier stuff looks good.

Comment 17

[Erik Vold [:erikvold] (please needinfo? me)]

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

Pointer to Github pull-request

Comment 18

[Erik Vold [:erikvold] (please needinfo? me)]

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

><!DOCTYPE html><meta charset="utf-8"><meta http-equiv="refresh" content="5;https://github.com/mozilla/addon-sdk/pull/1241"><title>Bugzilla Code Review</title><p>You can review this patch at <a href="https://github.com/mozilla/addon-sdk/pull/1241">https://github.com/mozilla/addon-sdk/pull/1241</a>, or wait 5 seconds to be redirected there automatically.</p>

Comment 19

[Erik Vold [:erikvold] (please needinfo? me)]

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

I made some updates to answer Blair's questions, and still need a review from Irakli.

Comment 20

[Erik Vold [:erikvold] (please needinfo? me)]

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

Asking for a review from Irakli now instead.

Comment 21

[Erik Vold [:erikvold] (please needinfo? me)]

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

Ready for reviews.

Comment 22

[Dave Townsend [:mossop]]

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

I don't think I have any further issues with this at this point

Comment 25

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

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

Erik, I'm +1 on general design, I think there are lot of details in the JEP
right now that is worth removing. And few things I disagree on, which is why
I r- this. Mainly I don't think we need any `options.json`, I believe we have
a good strategy with `jpm` now and I don't see a reason why it could not work
in a case of AOM.

As a side note, I think it would be best to keep bootstrap.js in AOM minimalistic, and move most of the work to add-on/runner module, which will
leave let us to keep iterating faster.

Comment 26

[Blair McBride [:Unfocused] (UNAVAILABLE)]

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

(Not sure why this was still in my queue - don't think there's anything more to add.)

Comment 27

[Erik Vold [:erikvold] (please needinfo? me)]

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

Hey Irakli,

I've updated the jep to remove all mentions of the options.json file and I'm mentioning using preferences instead now, does this look good?

Comment 28

[Erik Vold [:erikvold] (please needinfo? me)]

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

oops meant to r? Irakli

Comment 29

[Erik Vold [:erikvold] (please needinfo? me)]

We should have our test suites passing using toolkit/loader before we can ship native jetpacks.

Comment 30

[Firefox Bug Husbandry Bot]

https://bugzilla.mozilla.org/show_bug.cgi?id=1399562