Re: [Tails-dev] Please review feature/use_ferm

2012-09-24 Thread Ague Mill
On Mon, Sep 24, 2012 at 03:56:05PM +0200, berta...@ptitcanardnoir.org wrote:
> > I'm not too happy with the initial commit (f00effb), because it
> > removes the check for the needed tool existence and leaves the exit
> > code checking to the implicit.

Fixed in devel.

> > I suggest:
> > 
> >   * re-adding something like:
> > [ -x /usr/sbin/ferm ]  || exit 2
> > 
> >   * clarifying with a comment that the ferm command invocation should
> > remain the last one in this script.

I went with the `set -e` way instead of that last one.

> > About ferm.conf, the Emacs mode line sets shell-script, but given the
> > syntax, apparently conf-space-mode or perl-mode do a quite better job,
> > so I suggest:
> > 
> >   # -*- mode: conf[space] -*-

Done in devel.

-- 
Ague


pgpIoqnxf2CJT.pgp
Description: PGP signature
___
tails-dev mailing list
tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev


Re: [Tails-dev] Please review feature/use_ferm

2012-09-24 Thread bertagaz
Hi,

On Mon, Sep 24, 2012 at 12:27:59PM +0200, intrigeri wrote:
> Hi,
> 
> > Reviews welcome, candidate for the next major release.
> 
> I'm not too happy with the initial commit (f00effb), because it
> removes the check for the needed tool existence and leaves the exit
> code checking to the implicit.
> 
> I suggest:
> 
>   * re-adding something like:
> [ -x /usr/sbin/ferm ]  || exit 2
> 
>   * clarifying with a comment that the ferm command invocation should
> remain the last one in this script.
> 
> About ferm.conf, the Emacs mode line sets shell-script, but given the
> syntax, apparently conf-space-mode or perl-mode do a quite better job,
> so I suggest:
> 
>   # -*- mode: conf[space] -*-
> 
> Other than that, "static" reviews passes as far as I'm concerned,
> and I'll test and merge this later today or tomorrow.

Oops, I was doing the test and merged the branch in devel as the firewall
configuration doesn't seem to have changed with this feature. Then I saw
your email... Too late, already pushed it :/

Feel free to revert the merge, or do another merge if/when intrigeri
suggestions are implemented.
 
> Nice job, anonym and ague!

+1

bert.
___
tails-dev mailing list
tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev


Re: [Tails-dev] Please review feature/use_ferm

2012-09-24 Thread intrigeri
Hi,

> Reviews welcome, candidate for the next major release.

I'm not too happy with the initial commit (f00effb), because it
removes the check for the needed tool existence and leaves the exit
code checking to the implicit.

I suggest:

  * re-adding something like:
[ -x /usr/sbin/ferm ]  || exit 2

  * clarifying with a comment that the ferm command invocation should
remain the last one in this script.

About ferm.conf, the Emacs mode line sets shell-script, but given the
syntax, apparently conf-space-mode or perl-mode do a quite better job,
so I suggest:

  # -*- mode: conf[space] -*-

Other than that, "static" reviews passes as far as I'm concerned,
and I'll test and merge this later today or tomorrow.

Nice job, anonym and ague!
___
tails-dev mailing list
tails-dev@boum.org
https://mailman.boum.org/listinfo/tails-dev