11/07/14 02:10, intrigeri wrote:
> Hi,
>
> anonym wrote (10 Jul 2014 23:05:49 GMT) :
>> [...] so IMHO only a code review is necessary, which I've assigned
>> to intrigeri.
>
> Commit fe9da04 is a pain to review, as it mixes renaming files, with
> changing their content. So, I'm manually diff'ing the old files with
> the new ones.
Sorry, didn't think about that. OTOH it didn't feel like it would make
sense to rename the old directory to wheezy in a separate commit... or
do you think it's worth that, for future reference?
>> - :sudo_cmd => "echo '%p'|sudo -S sh '%f'",
>> + :sudo_cmd => "echo '%p'|sudo -S bash '%f'",
>
> Why?
That's what the Veewee maintainer thinks, apparently. I just copied in
the Wheezy 7.5 definition.
>> +# Use http.debian.net to select closest mirror
>> +d-i mirror/country string manual
>> +d-i mirror/http/hostname string http.us.debian.org
>
> ... feels wrong.
Fixed the comment.
>> +gpg -a --export C7988EA7A358D82E | sudo apt-key add -
>
> Just curious: is armored output really needed?
No, so it's just unnecessary. Removed.
> The rest is about the documentation:
>
>> - # gem install --no-ri --no-rdoc veewee
>> [...]
>> + sudo gem install --no-ri --no-rdoc veewee
>
> I suspect this can create problems when run with a strict umask.
>
> I think it should be minimally documented why one still needs a clone
> on veewee's Git, after installing the Gem.
I don't know why, but it didn't work for me to run it form the vagrant
sub-directory of our sources before, but now it does. Adapted.
>> +define defined in `vagrant/Vagrantfile`.
>
> One too much word?
Fixed!
>> +Do not forget to update the SHA256 sum in
>> +`vagrant/lib/tails_build_settings.rb`.
>
> Isn't that what a sed command above does?
Forgot to remove it, now fixed.
> Does #5498 still apply? The notes about it were removed, but we still
> instruct to install the gem as root, so I'm wondering.
I did try `gem --user` earlier, which said there was no such option.
Apparently it is `gem install --user`, duh! Adapted.
>> -After issuing that command, Veewee will download the boot ISO image,
>> -drive the debian-installer using [preeseding] and run `postinstall.sh` that
>> -will take care of seting up the environment expected by Vagrant.
>> -
>> -[preseeding]: https://www.debian.org/releases/stable/i386/apbs02.html
>> -
>> -In order to support local HTTP proxy, the `preseed.cfg` is generated from an
>> -ERB template during the Rake task `create_preseed_cfg`.
>> -
>> -Once the initial setup is done, it is worthwhile to see if the *basebox*
>> -fits Vagrant requirements. Veewee ships with an automated test suite:
>> -
>> - $ vagrant basebox validate squeeze
>
> It looks like perhaps some functionality was dropped (which is fine),
> but perhaps other bits are still relevant? (at least the part that
> explains what Veewee does, and points to the preseeding doc)
Ok, added some explanations, some revived, some new.
Cheers!
