kurono:
> Hi Sajolida,
>
> Ok, I have applied all the suggested changes, and I think its time to a
> code review as well:
>
> https://git-tails.immerda.ch/kurono/liveusb-creator/log/?h=feature/9005-Improve-tails-installer
So I finally found the time to review this. Sorry for the huge delay!
And thanks for the testing tips. I also realized that they have to be
run with sudo (at least from Tails), so what about adding these
instructions to the repo?
git clone
https://git-tails.immerda.ch/liveusb-creator
sudo apt-get install python-distutils-extra python3-distutils-extra
sudo python setup.py install
sudo mkdir /usr/local/share/tails-installer
sudo cp -R data/* /usr/local/share/tails-installer/
sudo tails-installer
I'm sending a few screenshots in attachment for reference.
Regarding my tests and review:
1. I propose defaulting to "Clone" instead of "Use ISO" when running
from Tails because I expect it to be the most frequently used option
(and that's why it's on top).
2. What about deactivating (graying out) the file selection unless "Use
ISO" is selected? See:
https://mailman.boum.org/pipermail/tails-ux/attachments/20160629/62e60034/attachment-0003.png
3. If "Use ISO" is selected but no ISO image is specified, I could still
click "Install" (but then I don't know what it's doing). I think we
should then display the dialog for selecting the ISO image before asking
for confirmation before install. This would be the "forgiving" option.
Another option would be to add a constrain and gray out the "Install"
button until an ISO image is selected. But I think I prefer the first
option.
4. Did you implement changing the "Install" button into "Upgrade" (and
"Reinstall")? Because I didn't see this when doing my tests...
Now that I have a testing setup and manage to do this once, I should be
able to do more reviews faster :)
,%20so%20what%20about%20adding%20these%0A>%20instructions%20to%20the%20repo?%0A>%20%0A>%20git%20clone%20https://git-tails.immerda.ch/liveusb-creator%0A>%20sudo%20apt-get%20install%20python-distutils-extra%20python3-distutils-extra%0A>%20sudo%20python%20setup.py%20install%0A>%20sudo%20mkdir%20/usr/local/share/tails-installer%0A>%20sudo%20cp%20-R%20data/*%20/usr/local/share/tails-installer/%0A>%20sudo%20tails-installer%0A>%20%0A>%20I'm%20sending%20a%20few%20screenshots%20in%20attachment%20for%20reference.%0A>%20%0A>%20Regarding%20my%20tests%20and%20review:%0A>%20%0A>%201.%20I%20propose%20defaulting%20to%20%22Clone%22%20instead%20of%20%22Use%20ISO%22%20when%20running%0A>%20from%20Tails%20because%20I%20expect%20it%20to%20be%20the%20most%20frequently%20used%20option%0A>%20(and%20that's%20why%20it's%20on%20top).%0A>%20%0A>%202.%20What%20about%20deactivating%20(graying%20out)%20the%20file%20selection%20unless%20%22Use%0A>%20ISO%22%20is%20selected?%20See:%0A>%20%0A>%20https://mailman.boum.org/pipermail/tails-ux/attachments/20160629/62e60034/attachment-0003.png%0A>%20%0A>%203.%20If%20%22Use%20ISO%22%20is%20selected%20but%20no%20ISO%20image%20is%20specified,%20I%20could%20still%0A>%20click%20%22Install%22%20(but%20then%20I%20don't%20know%20what%20it's%20doing).%20I%20think%20we%0A>%20should%20then%20display%20the%20dialog%20for%20selecting%20the%20ISO%20image%20before%20asking%0A>%20for%20confirmation%20before%20install.%20This%20would%20be%20the%20%22forgiving%22%20option.%0A>%20Another%20option%20would%20be%20to%20add%20a%20constrain%20and%20gray%20out%20the%20%22Install%22%0A>%20button%20until%20an%20ISO%20image%20is%20selected.%20But%20I%20think%20I%20prefer%20the%20first%0A>%20option.%0A>%20%0A>%204.%20Did%20you%20implement%20changing%20the%20%22Install%22%20button%20into%20%22Upgrade%22%20(and%0A>%20%22Reinstall%22)?%20Because%20I%20didn't%20see%20this%20when%20doing%20my%20tests...%0A>%20%0A>%20Now%20that%20I%20have%20a%20testing%20setup%20and%20manage%20to%20do%20this%20once,%20I%20should%20be%0A>%20able%20to%20do%20more%20reviews%20faster%20:)%0A>%20 "Reply to this message")
