Refresh the whole page, add install and remove hooks #98

Merged
merged 4 commits into from Sep 14, 2017

Conversation

Projects
None yet
3 participants
Collaborator

caldav commented Sep 8, 2017

  • Refactor configure section
  • Add install and remove hooks
  • Add full example for configure
  • Try to adhere to the scriptlets page style which has been plebiscited by users
Collaborator

caldav commented Sep 8, 2017

@stolowski can you review please? If you have relevant examples to add, please do!

+# This could be downloading a list of dynamic configuration options or creating a database.
+```
+
+<br>
@caldav

caldav Sep 8, 2017

Collaborator

This is expected, it prettifies a CSS issue with titles following a code block.

Looks good overall, a few comments to consider. Thanks!

build-snaps/hooks.md
+
+There are three hooks types:
+
+* [`configure`](#configure): on demand hook to `snap get` or `snap set` a configuration value inside a snap.
@stolowski

stolowski Sep 8, 2017

Also executed during installation and refresh.

build-snaps/hooks.md
+There are three hooks types:
+
+* [`configure`](#configure): on demand hook to `snap get` or `snap set` a configuration value inside a snap.
+* [`install`](#install): run after a snap is installed on a system.
@stolowski

stolowski Sep 8, 2017

it's actually executed as part of the installation (during it).

build-snaps/hooks.md
+[...]
+```
+
+In this example, we have an application requesting hardware information during install or subsequent configuration. We are therefore granting access to the `hardware-observe` interface to the install and configure hooks, but denying them access to the `x11` one since they don't need to display anything on screen.
@stolowski

stolowski Sep 8, 2017

This is tricky... Interfaces that are not-autoconnected will not be available at the time the hook is executed.

build-snaps/hooks.md
+
+### “install”
+
+The install hook is called upon initial install. If the hook exits non-zero, the installation of the snap will fail.
@stolowski

stolowski Sep 8, 2017

Also worth mentioning is the fact, that 'install' hook is exectued before services of the snap are started (contrary to configure hook, which runs afterwards).

build-snaps/hooks.md
+
+### “remove”
+
+The remove hook is called upon snap removal.
@stolowski

stolowski Sep 8, 2017

It's called when the last revision of a snap gets removed.

Collaborator

caldav commented Sep 12, 2017

@stolowski updated

Looks goods, just two minor issues to fix and should be good to go. Thanks!

build-snaps/hooks.md
+```
+
+In this example, we have an application requesting hardware information during install or subsequent configuration. We are therefore granting access to the `hardware-observe` interface to the install and configure hooks, but denying them access to the `x11` one since they don't need to display anything on screen. Note that interfaces that are not auto-connected will not be available through hooks until they are connected.
+
@stolowski

stolowski Sep 14, 2017

Right. In that light I think the above example may be slightly confusing.

build-snaps/hooks.md
+
+Before running the `snapcraft` command to build your snap, this `install` file should be placed at this location:
+
+`<snap project>/snap/hooks/configure`.
@stolowski

stolowski Sep 14, 2017

'install', not configure.

build-snaps/hooks.md
+There are three hooks types:
+
+* [`configure`](#configure): run after snap installation, refresh and on demand to `snap get` or `snap set` a configuration value inside a snap.
+* [`install`](#install): run when a snap is installed on the system, before any services contained by the snap have been started.
@evilnick

evilnick Sep 14, 2017

Does this only run on initial install, and not for example, when the snap is refreshed? I think we should explicitly clarify that here

Thanks David, looks good to me!
Nb, there is some more fancy stuff one can do with snap configuration as get/set supports JSON values, e.g. you can have a map and then access a value inside it with dotted path such as snap abc get foo.bar.baz. But I think this is a meterial for next update.

@caldav caldav merged commit defb840 into master Sep 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment