What about prompting during plugin install instead of prepare? "This
plugin has hooks, would you like continue? Y/N"
I'm concerned about being prompted on each prepare, especially in the face
of --watch type scripts. I'm also not sure that plugin authors should
worry about supporting the "no" use case. If a hook is optional, you can
split it out into a separate plugin that is optionally installed. If its
not optional, you should probably not install the plugin at all if you
don't want to run the hook.
WDYT?
-Michal
On Fri, Jul 18, 2014 at 4:25 PM, Andrew Grieve <agri...@chromium.org> wrote:
> Finally got to having a look. Lots of neat stuff in there! Some specific
> feedback:
>
> - Not a fan of "<script>", "<hook>" might be better
> - context.commonModules is a great idea, but I don't think exposing
> requireCordovaModule or elementtree is a good idea.
> - E.g. what if we want to drop our dependency on elementtree?
> - I think it's important that we only expose cordova-lib's public API to
> hooks.
> - For the same reason, let's not expose cordovaUtil. just make the
> top-level "cordova" object available. If there are other APIs that are
> needed, we should add them to the public API.
> - Since you're making a new class, please don't call it "Hooker"
> ("HookManager", "HookRunner", etc)
> - Looks like you already added "ScriptsRunner.js"
> - Don't capitalize the file unless it exports as constructor
> - Maybe Hooker can be combined with ScriptsRunner?
>
> Supporting node hooks and adding plugin hooks are two big changes. Didn't
> look to see how the commits had things split up, but I'd like to see one
> change go in separately from the other.
>
> I'm a bit concerned that plugin hooks won't work well due to it being hard
> to write x-platform, server builds like PGBuild, Telerik won't want to have
> hooks run on their servers, Thym likely can't support it.
> Security-wise, I'd feel better about it if we prompted the user before
> enabling plugin scripts to run. Just a simple Y/N confirmation would be
> enough. E.g.: "Plugin org.foo.bar contains an installation hook. Allow it
> to run?"
>
>
>
>
> On Fri, Jul 11, 2014 at 3:42 AM, Sergey Grebnov (Akvelon) <
> v-seg...@microsoft.com> wrote:
>
> > Updated docs[1] as per review. Looking forward to community feedback
> > regarding:
> > 1 . Idea of defining hook scripts via config.xml and plugin.xml
> > 2. New Script Interface for .js files (not used for /hooks scripts for
> > backward compatibility)
> >
> > [1]https://github.com/apache/cordova-lib/pull/55
> > [2]
> >
> https://github.com/MSOpenTech/cordova-lib/blob/CB-6481-hooks/cordova-lib/templates/hooks-README.md#script-interface
> >
> > Thx!
> > Sergey
> > -----Original Message-----
> > From: Shazron [mailto:shaz...@gmail.com]
> > Sent: Wednesday, July 9, 2014 9:26 PM
> > To: dev@cordova.apache.org
> > Subject: Re: Proposal: hooks support for plugins
> >
> > Ah ok. However I think since it is effectively deprecated we should not
> > advertise the old way as a supported way to do things but add another
> > section saying that support for "./cordova/hooks" may be removed in the
> > future, and perhaps give a timeline for removal.
> >
> > On Wed, Jul 9, 2014 at 10:15 AM, Josh Soref <jso...@blackberry.com>
> wrote:
> > > Shazron wrote:
> > >> from the Cordova Hooks doc:
> > >> "This directory used to exist at .cordova/hooks, but has now been
> > >> moved to the project root."
> > >> -> doesn't this imply that .cordova/hooks
> > >> does not work anymore, you should use hooks/ ?
> > >
> > > I believe it's supported for backwards compatibility (just as
> > > www/config.xml is)
> >
>
