Code review comment for lp:~frankban/lpsetup/interactive-execution

Revision history for this message
Francesco Banconi (frankban) wrote :

Benji, I updated this branch following you suggestions:

- fIxed steps' descriptions indentation and formatting, e.g.::

$ ./lp-setup init-host --dry-run
This command will perform the following actions:

* Update your system and install necessary deb packages (ssh bzr apache2.2-common).
* Create the user frankban if it does not exist.
* Create Apache document roots for launchpad and enable required Apache modules (proxy proxy_http rewrite ssl
  deflate headers).
* Set up hosts file for Launchpad (/etc/hosts).
* Set up the user's ssh directory (/home/frankban/.ssh).
* Create, if it does not exist, the ssh key /home/frankban/.ssh/id_rsa and authorize this key for the user
  frankban.
* Add bazaar.launchpad.net to known hosts.
* Set up bazaar authentication: Francesco Banconi <email address hidden>.
* Set up Launchpad user id: frankban.
* Add required APT repositories and install Launchpad dependencies: launchpad-database-dependencies-9.1 launchpad-
  developer-dependencies apache2 apache2-mpm-worker libapache2-mod-wsgi.

- descriptions are now wrapped at terminal size
- simplified the confirm function, updated the corresponding tests
- s/dry/dry_run

Also updated the descriptions handling, in particular:
- an error is raised if placeholders are missing in the steps' descriptions
- an error is raised for missing descriptions
These changes convinced me to implement a --dry-run smoke test, so that we can ensure our steps are correctly described.

- *interactive* now is always an attribute of namespace, even if the sub command does not support interactive execution. => Removed the is_interactive method.

« Back to merge proposal