Closed Bug 915376 (native-jetpack) Opened 8 years ago Closed 4 years ago

[tracking] Native Jetpack

Categories

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

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: evold, Assigned: evold, NeedInfo)

References

Details

(Whiteboard: [status:inflight])

Attachments

(1 file, 1 obsolete file)

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

Feedback?
Attachment #804776 - Flags: feedback?(rFobic)
Attachment #804776 - Flags: feedback?(jsantell)
Attachment #804776 - Flags: feedback?(dtownsend+bugmail)
Comment on attachment 804776 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1241

adsfdf
Attachment #804776 - Flags: feedback?(jgriffiths)
Attachment #804776 - Flags: feedback?(jsantell)
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.
Attachment #804776 - Flags: feedback?(rFobic) → feedback-
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!
Attachment #804776 - Flags: feedback?(jgriffiths) → feedback-
Assignee: nobody → evold
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.
Attachment #804776 - Flags: feedback?(dtownsend+bugmail) → feedback+
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.
Attachment #804776 - Flags: feedback?(rFobic)
Attachment #804776 - Flags: feedback?(dtownsend+bugmail)
Attachment #804776 - Flags: feedback-
Attachment #804776 - Flags: feedback+
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.
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.
Depends on: 935290
Depends on: 935233
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
Attachment #804776 - Flags: feedback?(rFobic)
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
Attachment #804776 - Flags: feedback?(dtownsend+bugmail) → feedback+
(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?
Flags: needinfo?(rFobic)
(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!
Depends on: 745857
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.
Attachment #804776 - Flags: feedback?(rFobic)
Attachment #804776 - Flags: feedback?(jsantell)
Attachment #804776 - Flags: feedback?(jgriffiths)
Attachment #804776 - Flags: feedback?(dtownsend+bugmail)
Attachment #804776 - Flags: feedback?(bmcbride)
Summary: JEP AOM Native Jetpack Support → JEP Native Jetpack
Attachment #804776 - Flags: feedback?(jsantell) → feedback+
Attachment #804776 - Flags: feedback- → feedback+
Attachment #804776 - Flags: feedback?(jgriffiths) → feedback+
Not sure how much more positive feedback I can give on this. Erik - great job!
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.
Attachment #804776 - Flags: feedback?(dtownsend+bugmail) → feedback+
Attachment #804776 - Flags: feedback?(bmcbride) → feedback+
Depends on: 903039
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>
Attachment #8349144 - Attachment is obsolete: true
Blocks: 852271
No longer blocks: nodeification
Depends on: 786280, nodeification
Depends on: 951820
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.
Attachment #804776 - Flags: feedback+ → feedback?(bmcbride)
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.
Attachment #804776 - Flags: feedback?(rFobic) → review?(rFobic)
Comment on attachment 804776 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1241

Ready for reviews.
Attachment #804776 - Flags: review?(dtownsend+bugmail)
Attachment #804776 - Flags: review?(bmcbride)
Attachment #804776 - Flags: feedback?(bmcbride)
Flags: needinfo?(rFobic)
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
Attachment #804776 - Flags: review?(dtownsend+bugmail) → review+
No longer blocks: 852271
Duplicate of this bug: 852271
Duplicate of this bug: 907184
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.
Attachment #804776 - Flags: review?(rFobic) → review-
Depends on: 1009328
Depends on: 1009332
Depends on: 1009334
Depends on: 1009336
No longer depends on: 1009328
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.)
Attachment #804776 - Flags: review?(bmcbride) → review+
Depends on: 1024791
Depends on: jpm
Depends on: 1040238
Depends on: 1040270
Depends on: 1040276
Depends on: 1037235
Depends on: 1040432
Alias: native-jetpack
Depends on: 1042239
Depends on: 1038381
Depends on: 1039880
Whiteboard: [status:inflight]
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?
Attachment #804776 - Flags: review- → review?
Comment on attachment 804776 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1241

oops meant to r? Irakli
Attachment #804776 - Flags: review? → review?(rFobic)
Attachment #804776 - Flags: review?(rFobic) → review+
Depends on: 864158
Depends on: 1067331
Depends on: 952548
Depends on: 1077372
No longer depends on: nodeification
No longer depends on: 1009336
No longer depends on: 952548
No longer depends on: 786280
Blocks: 745857
No longer depends on: 745857
We should have our test suites passing using toolkit/loader before we can ship native jetpacks.
Depends on: sdk-travis
No longer blocks: sdk-feature
No longer blocks: 620553
No longer blocks: 856968
No longer blocks: 811560
Depends on: 1107650
Depends on: 935113
No longer blocks: 896029
Depends on: 971249
Summary: JEP Native Jetpack → [tracking] Native Jetpack
Depends on: 1075448
Depends on: 1025315
Blocks: 1123428
Depends on: 1050422
Depends on: 1131926
Flags: needinfo?(davidramos09)
Priority: P1 → --
https://bugzilla.mozilla.org/show_bug.cgi?id=1399562
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.