Tails

#1 Updated by sajolida 2015-11-01 10:55:35

• Assignee changed from sajolida to anonym
• QA Check set to Ready for QA
• Feature Branch set to doc/10303-qrcode-scripts

Done, please review. Assigning to anonym as RM and shell expert! There’s no hurry on this, so feel free to postpone if you prefer.

#2 Updated by anonym 2015-11-01 11:34:00

• Assignee changed from anonym to sajolida
• % Done changed from 0 to 50
• QA Check changed from Ready for QA to Dev Needed

--- a/wiki/src/contribute/how/documentation/compress-image.sh
+++ b/wiki/src/contribute/how/documentation/compress-image.sh
[...]
+    mat $image

It seems that mat should be listed as a dependency of compress-image.sh.

--- /dev/null
+++ b/wiki/src/contribute/how/documentation/qrcode-decode.sh
[...]
+for code in $* ; do

Please use $@ instead of $*, as it handles parameter quoting properly. Same is true for the other new script, wiki/src/contribute/how/documentation/qrcode-encode.sh.

#3 Updated by sajolida 2015-11-03 09:59:10

• Priority changed from Normal to Elevated
• Target version changed from Tails_1.7 to Tails_1.8

Postponing and raising priority.

#4 Updated by sajolida 2015-11-04 12:22:41

• Assignee changed from sajolida to anonym
• QA Check changed from Dev Needed to Ready for QA

Done all this and more in a6169fb..b4eb05b. Thanks for the scripting tips!

#5 Updated by sajolida 2015-11-13 01:50:12

• Affected tool set to Installation Assistant

#6 Updated by intrigeri 2015-12-06 16:25:41

• Assignee changed from anonym to sajolida
• QA Check changed from Ready for QA to Dev Needed

anonym wrote:
> Please use $@ instead of $*, as it handles parameter quoting properly. Same is true for the other new script, wiki/src/contribute/how/documentation/qrcode-encode.sh.

I believe this bonus feature of $@ requires double quoting it, see the “Special Parameters” in dash(1) for details.

The $image variables need to be quoted.

I would understand that such painful aspects of shell piss you off. You already know what I recommend on this topic, for your peace of mind and the one of all reviewers ;)

I don’t understand how I’m supposed to use qrcode-encode.sh:.

$ wiki/src/contribute/how/documentation/qrcode-encode.sh bla
wiki/src/contribute/how/documentation/qrcode-encode.sh: line 14: compress-image.sh: command not found
$ cd wiki/src/contribute/how/documentation
$ ./qrcode-encode.sh bla
./qrcode-encode.sh: line 14: compress-image.sh: command not found

=> please either fix it, or make it more user friendly, or document how we can use it :)

#8 Updated by sajolida 2015-12-11 14:29:41

• Assignee deleted (sajolida)
• QA Check changed from Dev Needed to Ready for QA

#10 Updated by intrigeri 2015-12-14 09:59:55

• Assignee set to anonym

#11 Updated by anonym 2015-12-16 14:06:18

• Target version changed from Tails_1.8 to Tails_2.0

#12 Updated by anonym 2016-01-22 14:40:00

• Assignee changed from anonym to sajolida
• % Done changed from 50 to 70

intrigeri wrote:
> > In contribute/how/documentation.mdwn there’s “To run these scripts you need […] to have the [[compress-image.sh]] script in your executable path.”. Did you see this?
>
> Indeed I had not seen this, thanks for the pointer! It feels a bit weird to me to address this with documentation rather than programmatically, and the resulting UX for whoever missed the doc is not that good, but I don’t want to over-emphasize this problem given what scripts we’re talking about :) I’ll let someone else review and merge or fix this as they prefer.

Indeed, it is a bit awkward. I pushed a fix which runs compress-image.sh from the same directory as qrcode-encode.sh lives in.

What do you think?

#13 Updated by sajolida 2016-01-24 16:07:39

• Status changed from Confirmed to Resolved
• % Done changed from 70 to 100

Applied in changeset commit:ce17f8b05983d89da19243c1e63e0da1942d24f6.

#14 Updated by sajolida 2016-01-24 16:08:15

• Status changed from Resolved to Confirmed
• Assignee deleted (sajolida)
• Priority changed from Elevated to Normal
• % Done changed from 100 to 70
• QA Check deleted (Ready for QA)

Cool, I’m merging then…

#15 Updated by sajolida 2016-01-24 16:08:41

• Status changed from Confirmed to Resolved

#16 Updated by intrigeri 2016-01-24 16:49:27

• Status changed from Resolved to In Progress
• Assignee set to sajolida

• The example commands in the doc say qrcode-encode and qrcode-decode, while the actual scripts have the .sh extension ⇒ reopening as I guess it’s a blocker.
• Now that the $PATH problem was resolved, maybe it would be nice if the documentation used the full path to the scripts, so that it works out-of-the-box instead of returning “command not found: qrcode-encode”. This is not a blocker, just a note in passing :)

#17 Updated by sajolida 2016-01-29 18:19:23

• Target version changed from Tails_2.0 to Tails_2.2

#18 Updated by sajolida 2016-02-21 20:17:42

• Assignee changed from sajolida to anonym
• QA Check set to Ready for QA

Fixed the .sh naming in b11b0dc. Please review and merge again.

I don’t understand intrigeri’s comment on “using the full path to the scripts” but honestly, seeing what’s at sake here I don’t want to spend more time on this.

#19 Updated by anonym 2016-02-24 14:16:54

• Status changed from In Progress to Fix committed
• Assignee deleted (anonym)
• % Done changed from 70 to 100
• QA Check changed from Ready for QA to Pass

I fixed the last issues. Sajolida, have a look at commit:f14d45f if you want to understand what intrigeri meant.

#21 Updated by anonym 2016-03-08 19:03:19

• Status changed from Fix committed to Resolved