Re: New package: ifstatus

[Ricardo Wurmus]

Hello Stefan,

thank you for the contribution!

I think this would best fit to the module in ‘networking.scm’.  Usually,
we use git to track changes and create patches from committed changes.
You could add your package recipe to ‘networking.scm’ and create a
commit with the message:

    gnu: Add ifstatus.
    
    * gnu/packages/networking.scm (ifstatus): New variable.

Then you can create a patch from your last commit by running

    git format-patch -1

and just send it to the mailing list.

Now some comments about the package.

> ;;; Copyright © 2014 Stefan Reichoer <address@hidden>

It’s 2015 here ;)

>    (source
>     (origin
>      (method url-fetch)
>      (uri (string-append "http://ifstatus.sourceforge.net/download/ifstatus-v";
>                          version ".tar.gz"))

This line is a little too long.  Also, you might be able to use
“mirror://sourceforge/” instead of this direct download URL.

>      (sha256
>       (base32
>        "045cbsq9ps32j24v8y5hpyqxnqn9mpaf3mgvirlhgpqyb9jsia0c"))
>      (modules '((guix build utils)))
>      (snippet
>       '(begin
>          (substitute* "Main.h"
>                       (("#include <stdio.h>") "#include <stdio.h>\n#include 
> <stdlib.h>"))))
>      ))

This is not indented as it should be.  How about this instead:

    (snippet
     '(substitute* "Main.h"
        (("#include <stdio.h>")
         "#include <stdio.h>\n#include <stdlib.h>")))

>    (arguments
>     '(#:tests? #f  ;no "check" target
>       #:phases
>       (modify-phases %standard-phases
>                      (delete 'configure)       ;; no configure script
>                      (replace 'install
>                               (lambda* (#:key outputs #:allow-other-keys)
>                                 (let* ((out (assoc-ref outputs "out"))
>                                        (bin (string-append out "/bin")))
>                                   (mkdir-p bin)
>                                   (copy-file "ifstatus"
>                                              (string-append bin 
> "/ifstatus"))))))))

Also here the indentation is off.  Please align the opening parentheses
of the ‘(delete ...)’ and ‘(replace ...)’ under the ‘o’ of
‘(modify-phases ...)’.  Furthermore, please only use one semicolon for
margin comments like “; no configure script”.  Two semicolons are used
only for full line comments.

>     (description
>      "IFStatus was developed for Linux users that are usually in console mode.
> It is a simple, easy to use program for displaying commonly needed / wanted 
> statistics
> in real time about ingoing and outgoing traffic of multiple network 
> interfaces that is
> usually hard to find, with a simple and effecient view. It is the substitute 
> for
> PPPStatus and EthStatus projects.")

“Linux” users? ;)
s/effecient/efficient/
Please use double-spacing between sentences.

How about something like this instead:

     “IFStatus is a simple, easy-to-use program for displaying commonly
needed / wanted real-time traffic statistics of multiple network
interfaces, with a simple and efficient view on the command line.  It is
intended as a substitute for the PPPStatus and EthStatus projects.”

~~ Ricardo