Hi.

On Wed, Sep 09, 2015 at 07:23:42PM +0200, Javier Barroso wrote:
> Hello,
> 
> I would like to recieve feed back about a script which I have just
> upload to github.
> 
> https://github.com/i5513/apt-history-gui
> 
> It is working on my computer, but I cannot be sure it will work on
> other debian installations.
> 
> It is , for now a perl script which will present you dpkg.log and
> apt/history.log in a ncurse interface.
> 
> I'm not sure how :all architecture should be analyze. So any idea is welcome


Off the top of my head:

1) Please replace 'gui' with 'tui' in your README. Should help to avoid
some confusion.

2) Please document the need of installed Curses::UI::Common and
grep-status. My Debian installation lacks both, for example.

3) At least *some* kind of help text (preferably - a manpage) would be
welcome.

4) While we're at it, "apt-history.perl-curses-ui" could use explicit
license comment. As of now, it's unclear whenever you provide this
script on terms of GPL2 or GPL2+.

5) Checking for existence of 'grep-status' *before invoking* probably
would not hurt either. Using an absolute path to 'grep-status' is a good
idea too.

6) The message printed at line 93 contains a typo - 'possible' ->
'possibly'.

7) The message printed at line 202 is not English as far as I can tell.

8) This part's 'grep .' meaning currently escapes me:

  open my $grep_status,  "grep-status -n -FPackage '' ".
        " -s Package -s Architecture".
        " -s Version -s Status -n |".
        "grep . |";

9) This part:

  open my $hfh, '{
    zcat $(ls -rt /var/log/apt/history.log*gz)
    cat /var/log/apt/history.log;
    } |';

could use proper Perl opendir handling.

10) The case of terminal with less than 15 lines is not handled at all.

11) The code could use some trailing whitespace and tabulation cleanup :)

Reco

Reply via email to