[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#659498: RFS: solarpowerlog -- photovoltaic data logging



Hi Tobias,

As promised, here's my review of your package:

  - In debian/solarpowerlog.default, you define /etc/solarpowerlog as
    the default RUNDIR; what files is your program expecting to find
    there, HTML templates? If so, installing some default templates in
    /usr/share/solarpowerlog and pointing your program to that directory
    seems like a better option.

    You also note there that start-stop-daemon is taking care of running
    solarpowerlog in the background, but according to
    start-stop-daemon(8), "this is a last resort, and is only meant for
    programs that either make no sense forking on their own, or where
    it's not feasible to add the code for them to do this themselves";
    since your program can put itself in the background, you should
    rather use that possibility. This way, it will also make sense
    checking the exit status of start-stop-daemon in do_start in the
    init script, because if you use its '-b' option, it can't determine
    if the process failed to execute.

    And it'd be best to wrap lines at 80 columns.

  - Running uscan tells me:

      Processing watchfile line for package solarpowerlog...
      Newest version on remote site is 0.21a, local version is 0.22
      solarpowerlog: remote site does not even have current version

  - I think your short description should read "photovoltaic data
    logger", as in "solarpowerlog is a ..."

    And in the long description, you wrote "photo-voltaic"; I think you
    should stick to the former spelling.

    The second paragraph of the long description is missing punctuation.

  - In the solarpowerlog(1) man page, the SEE ALSO section appears to be
    a subsection of OPTIONS.

    Please consider removing the AUTHOR section (see man-pages(7) for an
    explanation why).

    In DESCRIPTION, I would start with "solarpowerlog tracks and
    logs..."; it's not just its purpose, it's what it actually does,
    right? Same for the second sentence.

    In OPTIONS, you start with "These programs"; why the plural?

Cheers,

-- 
Benoît Knecht



Reply to: