Author: Kill Your TV Date: To: tails-dev Subject: Re: [Tails-dev] [review'n'merge:1.2]
feature/7732-i2p-network-manager-hook
On Wed, 1 Oct 2014 00:45:10 +0000 (UTC)
anonym <anonym@???> wrote:
> 27/09/14 12:53, Kill Your TV wrote:
> > On Fri, 26 Sep 2014 16:25:16 +0000 (UTC) > As to your update to the manual test suite in commit dcc88ff, it again
> introduces interdependency to feature/7725-i2p-browser since it talks
> about the "I2P Browser". I was merely referring to the changes w.r.t.
> notifications when I2P is starting. In fact, I think you should
> rewrite the history of feature/7732-i2p-network-manager-hook and
> *move* commit dcc88ff into feature/7725-i2p-browser. I'll take care
> of these tests, and any potential merge conflicts by merging in
> commit dcc88ff.
Oops. (again...*sigh*). I cherry-picked dcc88ff ontop of the browser
branch.
>
> Please have a look at the feature/7732-i2p-network-manager-hook I just
> pushed (based on the commit before dcc88ff). It refactors your code
> quite heavily, and confines the logic into one place, so it's easier
> to follow. As for the new I2P shell library I believe its
> i2p_router_console_is_ready() can be handy in your tor-browser script
> instead of the custom `netstat ... | grep ...`.
Looks good. Having noticed the tor shell libs recently, this was
something I had in mind for > 1.2.
I cherry-picked the shell functions for the browser branch
and replaced that custom netstat call. I noticed, however, that the
wrong path to the new tails-i2p was added to refresh-translations in
4a2f4776e2ad. I corrected that (s/bin/sbin/) and pushed it.
I just noticed that TorButton forces the UA to 'TorBrowser'. I tacked
on a commit to work around that because seeing two 'Tor Browser's in
the task bar would probably be really confusing. Perhaps my idea is not
a dumb way of handling it.
> I'm sorry for not being very atomic with the commits, as I sneaked in
> some unrelated changes along the way in the "refactor" commit. Any
> way, what do you think? Would you merge this? :) BTW, I've personally
> tested this branch according to the new manual test steps of this
> branch.
That's *totally* fine and of course I'd merge it. :)
I'm sorry there's been so much refining/tweaking needed for these two
branches.