Pointer to Github pull-request

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

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

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 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 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 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.

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.

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 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

(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?

(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 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.

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.

Pointer to Github pull-request

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 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 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 on attachment 804776 [details] Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1241 Ready for reviews.

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 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 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 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 on attachment 804776 [details] Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1241 oops meant to r? Irakli

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