Hello, On Wed, Sep 9, 2015 at 7:43 PM, Reco <recovery...@gmail.com> wrote: > 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. Done, I have planned to make a gtk3 version
> > 2) Please document the need of installed Curses::UI::Common and > grep-status. My Debian installation lacks both, for example. > Done, thanks > 3) At least *some* kind of help text (preferably - a manpage) would be > welcome. > Done inline, for the moment > 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+. > Added at man page > 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 > Done > 6) The message printed at line 93 contains a typo - 'possible' -> > 'possibly'. > Thanks > 7) The message printed at line 202 is not English as far as I can tell. > Thanks, fixed > 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 . |"; > That is to erase "^$" lines, so I can play with module (%) operator > 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. > To be done > 10) The case of terminal with less than 15 lines is not handled at all. > mmm I'm not sure how to handle it > 11) The code could use some trailing whitespace and tabulation cleanup :) > I will review > Reco > Thank you very much!