Created attachment 189954 [details] [review]
shellDBus: Add 'LoadExtension' method

This allows clients to tell the shell to load extensions at runtime.

Created attachment 189955 [details] [review]
lookingGlass: Recognize new extensions as they are added

Given that this involves Extensions and LookingGlass territory, Walters, can you review this?

Created attachment 190823 [details] [review]
shellDBus: Add LoadExtension() method

This allows clients to tell the shell to load extensions at runtime.



Write a more conformant commit message, fix copy-paste error.

Created attachment 190824 [details] [review]
lookingGlass: Recognize new extensions as they are added

Fix white-space errors.

Review of attachment 190823 [details] [review]:

Looks good, should have some error handling though.

::: js/ui/shellDBus.js
@@ +14,3 @@
+              { name: 'LoadExtension',
+                inSignature: 's',
+                outSignature: ''

What about error handling?
It should probably return a boolean indicating success / failure.

@@ +62,3 @@
     },
 
+    LoadExtension: function(uuid) {

I'd add a doc comment in front off that.

Error handling is done separately: see bug #653208

Review of attachment 190824 [details] [review]:

Looks good, other then the naming nitpick which might require some re factoring to fix.

::: js/ui/lookingGlass.js
@@ +649,3 @@
+            this._loadExtension(uuid);
+
+        ExtensionSystem.signals.connect('extension-loaded',

This does not match any convention we use. i.e we don't have foo.signals.connect but just foo.connect. Fixing it would pretty much mean making the extensionSystem grow its own class. Not sure if it is worth it.

(In reply to comment #7)
> Error handling is done separately: see bug #653208

Well that means one has to:

1) Call LoadExtension
2) Call GetExtensionErrors to check whether there are any errors and get the actual messages.

Better would be:

1) Call LoadExtension and only call have to call GetExtensionErrors when there are errors.

Created attachment 191924 [details] [review]
shellDBus: Add LoadExtension() method

This allows clients to tell the shell to load extensions at runtime.



OK, I added something like this, and also made loadExtension respect enabled/disabled settings.

Review of attachment 191924 [details] [review]:

Looks good.

Marking rejected for various reasons. Sorry for wasting your time.

(In reply to comment #12)
> Marking rejected for various reasons. Sorry for wasting your time.

Can you please clarify?

The simple reason is that the plugin doesn't use this method, and I'm not planning on supporting the HTTP server approach. I'll have a replacement soon, I promise.

(In reply to comment #14)
> The simple reason is that the plugin doesn't use this method, and I'm not
> planning on supporting the HTTP server approach. I'll have a replacement soon,
> I promise.

Cool. Well please let me know (so I can support it in tweak-tool).