Autor: anonym Data: Para: The Tails public development discussion list Asunto: Re: [Tails-dev] [review'n'merge: 1.3]
bugfix/7951-refactor-chroot-browsers
On 13/12/14 11:43, intrigeri wrote: > Hi,
>
> anonym wrote (30 Nov 2014 20:42:02 GMT) :
>> On 31/10/14 13:55, intrigeri wrote:
>>> The design doc should be updated, at least to point to the new shell
>>> library file.
>
>> Indeed.
>
> I see some good progress in the branch.
> How about pointing to i2p.sh in contribute/design/I2P_Browser?
Done.
>>> Style nitpicking: I find it weird to on the one hand quote constant
>>> strings with double-quotes consistently (e.g.
>>> BROWSER_NAME="i2p-browser"), while on the other hand using constructs
>>> like /my/string/"$variable" in many places. I would either *not* quote
>>> constant strings ever, or write the latter "/my/string/$variable"
>>> instead. Preferably the latter. What do you think?
>
>> Well, sure.
>
> Good. I've gone a bit further with c9599e6.
Ah, I missed those. I fixed a little issue that introduced in 02f4261.
>>> Regarding commit 8051820d, this is an extra safety measure, useful
>>> only if the checking added in previous commit fails to error out,
>>> right? If so, I think adding a comment would be helpful.
>>>
>>> I see an undocumented set_key() function is added to common.sh.
>>> I think its name is much too generic for common.sh.
>
>> Hmm. True. What about `set_simple_config_key()`?
>
> Fine with me.
Done. Also documented it (and two other library functions).
