On 2 Nov 2015 18:58, "Bryan Quigley" <bryan.quig...@canonical.com> wrote: > > Thanks for the review Felipe > > > Why did you preserve runlevel S? I don't think this really belongs in > > recovery mode. > Changed > > >> + ;; > >> + stop|restart|reload|force-reload) > > > > restart (and force-reload?) should probably re-run the recovery script. > > Changed. > > > >> +Description=To recover nvi edit sessions. > > > > s/To r/R/ ? > +1 > > > Also, the init script Provides: nviboot, so you should provide a link > > from nviboot.service to nvi.service in /lib/systemd/system. > I changed it so it provided nvi. That was the whole reason I changed > the name afterall. > > >> ++if [ -n "$sessions_found" ] ; then > >> ++ echo "done." > >> ++else > >> ++ echo "none found." > >> ++fi > > > > This is a behavior change: previously the recover script would not > > print any output. Maybe this should not be printed? > > The previous init script did print none found though. > > > Why do you invoke dh_systemd_enable manually? the --with=systemd above > > should do it. > > > >> + dh_installinit -- start 70 S . > > > > start argument is obsolete now and just invokes `defaults`, so this > > override can go. > > Because I didn't know what i was doing, oops. > > > However, this changes the name of the init script from nviboot to nvi. > > This means you need to use dpkg-maintscript-helper to handle the > > rename, and remove the old enablement links as well. If you want to do > > this you might as well move it out of runlevel S ;) > > Moved out of runlevel S and it deletes nviboot correctly now. >
Also, you might want to remove the - from the su call. A login shell is not required. Saludos