Closed Bug 1007061 (rm-am) Opened 6 years ago Closed 5 years ago

Remove /browser/devtools/app-manager/

Categories

(DevTools Graveyard :: WebIDE, defect)

x86
All
defect
Not set
normal

Tracking

(firefox44 fixed)

RESOLVED FIXED
Firefox 44
Tracking Status
firefox44 --- fixed

People

(Reporter: paul, Assigned: ochameau)

References

Details

Attachments

(1 file, 5 obsolete files)

One the new app manager has landed, we want to remove the old code. We need to make sure the new code doesn't depend on the old code anymore.
Filter on 86b7095e-2bd0-499e-a704-d00f2524aeef / PAUL STOP SETTING QA CONTACT TO THE DEVTOOLS COMPONENT'S WATCHERS EMAIL FOR BUGS YOU FILE :)
QA Contact: developer.tools
Duplicate of this bug: 980919
Duplicate of this bug: 980335
And we need to ensure WebIDE is as much usage as app-manager from gaia developers perspective.
I think most issues have been highlighted in bug 1055279 comments.
A deprecation notice was added to App Manager in 35 that points you to WebIDE (bug 1070957).

I think it would be good to resolve bug 1055347 (searching for apps) before removing App Manager, as that was a vocal complaint from Gaia devs.

Other than that, we've migrated everything to WebIDE.

Can anyone think of other tasks to complete before removing App Manager?

I will be sure to announce it's impending removal before actually removing it.
Flags: needinfo?(poirot.alex)
Flags: needinfo?(paul)
Flags: needinfo?(janx)
Reducing the number of required clicks is important too. I think this bug should depend on bug 1079347.
Flags: needinfo?(paul)
(In reply to Paul Rouget [:paul] (slow to respond. Ping me on IRC) from comment #8)
> Reducing the number of required clicks is important too. I think this bug
> should depend on bug 1079347.

Right, I assumed "searching for apps" would also be achieved via that bug, but yes, let's depend on it as well.
Depends on: 1079347
I see two things, one "simple" the app selection you just mentioned and one another, harder, really specific to gaia devs.
That's about auto-connect to last runtime.
When you flash gaia with `make install-gaia`, or `APP=system make install-gaia`, it reboots b2g without reseting adb. The auto-connect feature doesn't work as the runtime stays on but we just loose the debugger connection.
With that behavior, in some cases, when working on gaia, you have to click more (2 additional clicks, involving a popup) each time you update your device. In the app manager you just had to hit connect once, without any popup.
I'm not sure it is mandatory to fix that, but that would be really nice to have.
Flags: needinfo?(poirot.alex)
(In reply to Alexandre Poirot [:ochameau] from comment #10)
> I see two things, one "simple" the app selection you just mentioned and one
> another, harder, really specific to gaia devs.
> That's about auto-connect to last runtime.
> When you flash gaia with `make install-gaia`, or `APP=system make
> install-gaia`, it reboots b2g without reseting adb. The auto-connect feature
> doesn't work as the runtime stays on but we just loose the debugger
> connection.
> With that behavior, in some cases, when working on gaia, you have to click
> more (2 additional clicks, involving a popup) each time you update your
> device. In the app manager you just had to hit connect once, without any
> popup.
> I'm not sure it is mandatory to fix that, but that would be really nice to
> have.

Okay, I will tentatively depend on that as well, since it's another version of the "too many clicks" problem.
Depends on: 1055278
App search would be a big plus, but else removing App Manager seems fine.

(I guess the files app-projects.js, app-validator.js, plugin.js, default-app-icon.png, rocket.svg and noise.png will be moved into the respective webide/ folders?)

One thing an app developer told me is that their team still use the old "Firefox OS 1.1 Simulator" addon, but they were happy about WebIDE in general. Also, they were having trouble debugging marketplace apps on desktop (I'll see what the problem is and file bugs if necessary).
Flags: needinfo?(janx)
Attached patch Remove app-manager. (obsolete) — Splinter Review
Here is a first step, remove everything but what is still used by WebIDE.
Attached patch patch v2 (obsolete) — Splinter Review
Cleanup even more stuff from browser/.
Attachment #8671363 - Attachment is obsolete: true
Attached patch patch v3 (obsolete) — Splinter Review
Rebased, already a (simple) conflict :x

But I still see an issue with WebIDE project editor being empty dependeing on file type.
Unfortunately I got an exception (with no stack!!! Trying to figure out why/how)
that just says "Error" (so the stack is even more needed!).
Attachment #8671393 - Attachment is obsolete: true
Attached patch patch v4 (obsolete) — Splinter Review
Rebased. Looks like the exception vanished... I may be based on a broken gecko changeset?

Looks like try is happy.
Do you think that's the right time to proceed?
Should we wait for about:debugging to support devices/simulators?

We can then followup by moving modules still used by webide
or stop using them.
Attachment #8671619 - Attachment is obsolete: true
Attachment #8673116 - Flags: feedback?(jryans)
Comment on attachment 8673116 [details] [diff] [review]
patch v4

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

::: devtools/client/app-manager/moz.build
@@ +7,4 @@
>  MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
>  
>  DevToolsModules(
>      'app-projects.js',

Do you intend to clean up things like this by moving to WebIDE in a separate step?

::: devtools/client/jar.mn
@@ -345,5 @@
> -    skin/themes/app-manager/images/plus.svg (themes/app-manager/images/plus.svg)
> -    skin/themes/app-manager/images/remove.svg (themes/app-manager/images/remove.svg)
> -    skin/themes/app-manager/images/add.svg (themes/app-manager/images/add.svg)
> -    skin/themes/app-manager/images/index-icons.svg (themes/app-manager/images/index-icons.svg)
> -    skin/themes/app-manager/images/rocket.svg (themes/app-manager/images/rocket.svg)

WebIDE uses rocket and noise[1]

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/themes/webide.css#137

@@ -347,5 @@
> -    skin/themes/app-manager/images/add.svg (themes/app-manager/images/add.svg)
> -    skin/themes/app-manager/images/index-icons.svg (themes/app-manager/images/index-icons.svg)
> -    skin/themes/app-manager/images/rocket.svg (themes/app-manager/images/rocket.svg)
> -    skin/themes/app-manager/images/noise.png (themes/app-manager/images/noise.png)
> -    skin/themes/app-manager/images/default-app-icon.png (themes/app-manager/images/default-app-icon.png)

WebIDE uses default-app-icon[1]

[1]: https://dxr.mozilla.org/mozilla-central/source/devtools/client/webide/modules/app-manager.js#33
Attachment #8673116 - Flags: feedback?(jryans) → feedback+
(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #21)
> Comment on attachment 8673116 [details] [diff] [review]
> patch v4
> 
> Review of attachment 8673116 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: devtools/client/app-manager/moz.build
> @@ +7,4 @@
> >  MOCHITEST_CHROME_MANIFESTS += ['test/chrome.ini']
> >  
> >  DevToolsModules(
> >      'app-projects.js',
> 
> Do you intend to clean up things like this by moving to WebIDE in a separate
> step?
> 

Yes, let's first prune the unnecessary files. Then see resource by resource if each is required and how to refactor or just move files around.
Just sent a note on devtools and fxos mailing list:
  https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/yRJbSzvsODg
I don't see it on dev-fxos yet, but I hope it will appear there also!
Assignee: nobody → poirot.alex
Attached patch patch v5 (obsolete) — Splinter Review
Attachment #8673116 - Attachment is obsolete: true
Attachment #8675601 - Flags: review?(jryans)
Just some more latency on fxos, here is the note:
  https://groups.google.com/forum/#!topic/mozilla.dev.fxos/883sAxUjDoc
Comment on attachment 8675601 [details] [diff] [review]
patch v5

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

Looks good to me! :D

If you don't plan to do it in this bug, please file a follow up to clean up the remaining files still used by WebIDE.
Attachment #8675601 - Flags: review?(jryans) → review+
Yes, I plan to do it in followups. I would like to just wait for you resource://devtools effort to land before landing this.
Blocks: 1216590
Attached patch patch v6Splinter Review
Rebased after the resource://devtools file move.
Attachment #8675601 - Attachment is obsolete: true
Attachment #8677371 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/c0732e791208
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Product: Firefox → DevTools
Product: DevTools → DevTools Graveyard
You need to log in before you can comment on or make changes to this bug.