Re: [Tails-dev] Please review & merge bugfix/less-aggressive-hard-disk-APM-on-AC
Alan wrote (13 Mar 2013 19:52:44 GMT) : > Merged thanks. Unmerged.. as I wrote on the 3rd, my merge request would be suspended if Maxim came up with additional information about why 254 would be bad, and this did happen. I've still some more research and tests to do on various hardware to finish convince myself that's nevertheless the best value for Tails on AC power. Stay tuned. ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Please review & merge bugfix/less-aggressive-hard-disk-APM-on-AC
On Wed, Mar 13, 2013 at 9:50 PM, Alan wrote: > I don't buy this position, as this branch is here to fix issues > appearing for some people that are fixed by -B254 but happening with > -B128. You don't understand the issue then. -B128 does not prevent spindown, whereas -B254 most likely does. -- Maxim Kammerer Liberté Linux: http://dee.su/liberte ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Please review & merge bugfix/less-aggressive-hard-disk-APM-on-AC
Hi, [...] > Tails developers: I've rewritten the branch history to use 128 instead > of 127, and improved the commit message to explain why. > Merged thanks. I didn't find a ticket, so I didn't close any. Cheers ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Please review & merge bugfix/less-aggressive-hard-disk-APM-on-AC
Hi, On Sun, 3 Mar 2013 08:15:48 +0200 Maxim Kammerer wrote: > On Sun, Mar 3, 2013 at 1:02 AM, intrigeri wrote: > > But well, I guess Maxim will come out with a few bug reports that > > clearly show that -B 254 is harmful ;) > > Well, if you insist… > https://bugs.launchpad.net/ubuntu/+source/acpi-support/+bug/59695/comments/286 > Thanks for the pointer [...] > > The problem is that -B254 will probably prevent drive spindown for > most people, so it's a tradeoff, and I don't think that one or even > several reports from users warrant turning power management off, > especially for a live distribution that does not require a hard drive. > Those users should file bugs against respective kernel modules. People > probably experience issues due to wireless power saving, etc., as well > — there is no one setting for any device that makes everyone happy. > I don't buy this position, as this branch is here to fix issues appearing for some people that are fixed by -B254 but happening with -B128. Cheers ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Please review and merge feature/about_tails
hi,
Alan wrote (13 Mar 2013 18:40:36 GMT) :
> Please review and merge the branch `feature/about_tails`.
Great. I see a few things that should be fixed, see bellow.
I think all translations should be preserved in the desktop file,
not only those for a half dozen half-randomly chosen languages.
> -Categories=GNOME;GTK;Core;Utility;
> +Categories=GNOME;GTK;Utility;
I'd rather not see changes made without a clear reason.
> +about_dialog.set_title=(_("À propos de Tails"))
This looks wrong to me.
> +about_dialog.set_copyright("Tails Developpers")
s/Delevoppers/developers/ for consistency reasons + fixing the typo.
I'll be happy to review (and test, this time) a fixed branch :)
Cheers,
--
intrigeri
| GnuPG key @ https://gaffer.ptitcanardnoir.org/intrigeri/intrigeri.asc
| OTR fingerprint @ https://gaffer.ptitcanardnoir.org/intrigeri/otr.asc
___
tails-dev mailing list
tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev
[Tails-dev] Please review and merge feature/about_tails
Hi, Please review and merge the branch `feature/about_tails`. The ticket is there: https://tails.boum.org/todo/about_Tails_launcher/ Cheers ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] Please review&merge bugfix/disable-indymedia-irc-account
On 12/03/13 16:54, intrigeri wrote: > Hi, > > ticket: https://tails.boum.org/todo/disable_indymedia_irc_account/ > candidate for 0.17.1 and 0.18, so please merge into stable and devel. > > Note that the ticket / decision were pretty unclear: that account is > already disabled by default anyway, so it can't be disabled much more, > so I interpreted the decision as removing the account altogether. Agreed: the solution was to disable it. > I've added a note on todo/wheezy to re-enable this account. signature.asc Description: OpenPGP digital signature ___ tails-dev mailing list tails-dev@boum.org https://mailman.boum.org/listinfo/tails-dev
Re: [Tails-dev] remembering installed packages (from TODO)
On 12/03/13 13:23, intrigeri wrote:
> Hi,
>
> Alan wrote (11 Mar 2013 21:15:00 GMT) :
>> I have a working basic implementation in
>> feature/remember_installed_packages. If you want to give a hand, please
>> have a look and/or test and give feedback.
>
> Here's a first quick "static" review.
>
>> /etc/NetworkManager/dispatcher.d/70-apt-upgrade.sh
>
> This filename is misleading: it suggests the script runs apt-get
> upgrade, which it (rightly) does not. I believe
> 70-upgrade-xyz-packages.sh would be a better name.
>
> One also would have to find something sensible to put in place of xyz.
> "persistent" is no valid candidate, since not all packages stored in
> persistence are upgraded by this script -- nor installed by the other,
> by the way, so the naming issue should be addressed everywhere else in
> this branch too.
>
>> +pp_log() {
>
> Please ensure the function names are unique in a nicer way than with
> a fixed prefix... like, by using more descriptive names (e.g.
> rename `pp_install' to `install_persistent_packages').
>
>> + # Log arguments to the logfile defined in $PP_LOGFILE
>
> Please document the function's purpose in a way that can't be confused
> with documenting the next shell command, e.g. by moving the function
> doc just before the function (same for most other functions, not just
> this one.)
>
>> + echo "$@" >> "$PP_LOGFILE"
>
> Please use syslog instead. See 20-time.sh for an example.
>
>> +pp_notify_user() {
>
> Please extract this function from 20-time.sh into
> usr/local/lib/tails-shell-library/$WHATEVER.sh, instead of duplicating
> the code.
>
>> + # Shows the paths of persistent packageslists
>> [...]
>> + # Shows the list of all the persistent packages
>
> "Shows" is a bit vague. "Print on standard output", perhaps?
>
> The aforementioned naming issue is obvious in "all the persistent
> packages".
>
>> + echo ${persistent_dir}${PP_PACKAGES_LIST_FILE}
>
> Any good reason not to quote the argument to echo?
>
>> + for package_list in $(pp_get_packages_lists); do
>> + cat "${package_list}"
>> + done
>
> "cat $(pp_get_packages_lists)" would be enough.
>
>> +pp_install_packages() {
>> + # Install packages given as arguments
>> + local package
>> + for package in ${@}; do
>
> I think running apt-get once for all remembered packages would be more
> efficient and more robust: it gives APT the big picture of what we
> want, instead of asking it to paint it blindly bit by bit.
>
>> + for package in ${@}; do
>
> Using ${} in this construct seems overkill to me, but I'd be happy to
> learn if it brings advantages I don't know about.
>
>> +apt-get --quiet --no-remove --yes install
>> "${package}" 2>&1
>
> I think you want -o 'DPkg::Options::=--force-confold' too.
>
$PP_LOGFILE || \
>
> Well, no, please use your logging function instead of gratuitously
> breaking abstraction layers you've setup.
>
>> + pp_install_packages "${packages}"
>
> Why pass the packages list as a single argument, while the
> pp_install_packages documentation reads "Install packages given as
> arguments"?
> ^
>
> Actually, I'm not sure pp_install_packages is worth existing at all as
> a separate function, but well.
>
>> + # The command which installs all persistent packages
>> [...]
>> +# The command which upgrades all persistent packages
>
> Same naming problem, again.
>
>> + echo "Will install the following packages: ${packages}"
>
> Why stdout instead of logging?
>
>> +pp_notify_user "`gettext 'Tails persistence'`" "`gettext 'Tails is
>> starting to upgrade persistent software.'`"
>
> I believe gettext was not initialized yet, so this has little chances
> to work. I'll be happy to review a further, tested version.
>
> Also, I'm not convinced about all the strings displayed to the user,
> and I'd like sajolida to have a look at it and suggest improvements.
Yesterday, we proposed to use the term "your additional software". So
that could become:
« Your additional software
Upgrading and installing your additional software... »
We're already using the gerund form in the notification of the time
synchronization, so let's stick to that.
« Your additional software
The upgrade was successful. »
« Your additional software
The upgrade failed. [Hint] »
Whenever the upgrade fails we should provide some more information to
the user, that could include the following, maybe using a link to the doc:
- more info or hints about why it failed
- hints about what else could be tried to do the upgrade: check the
network connection? restart?
But I wouldn't like to be left with just a failure message...
>> +apt-get --quiet update 2>&1 >> $PP_LOGFILE && \
>
> log function, here too.
>
>> +pp_notify_user "`gettext 'Tails persistence'`" "`gettext
>> 'Tails failed to upgrade persistent software.'
