Code review comment for ~dilyn-corner/chromium-browser/+git/snap-from-source:daemon

Revision history for this message
Dilyn (dilyn-corner) wrote :

> 1. I don't see any statement to set daemon to true, only to false. Why is that?

The ultimate intention here is to provide a way of running chromium as a proper kiosk. This is probably not the preferred way of running chromium, so we want to safeguard this as an opt-in choice for users of the snap. In addition, people who want to run chromium as a kiosk will probably want to ensure it launches automatically without intervention. Thus, the onus should be on users to explicitly say they want the kiosk feature, and only then should the daemon be enabled.

One use-case is for Ubuntu Core users: they can add a config value to their gadget snap to set daemon=true so that when they install the image on their device, chromium will launch in the right way "out of the box".

> And why do we need to set it to false if (as far as I know) nothing checks whether it is false?

We set it to false at the very least as a sanity check for users - it offers a transparent way to tell if the daemon is actually enabled or not.

> 2. Do we need both an install and a configure hook?

That's a fair question. Examples are provided by us here:
https://github.com/MirServer/iot-example-graphical-snap

Their install hook explicitly checks for whether or not the snap is installed on an Ubuntu Core system. I see no real need for this, as the question can be side-stepped in a simpler way (namely, setting the daemon as enabled via the gadget snap as mentioned above). I think we could safely eliminate the install hook for the reason you point out; I included it here for a more "complete" handling of the snap during its life cycle.

« Back to merge proposal