On Tue, Jun 23, 2015 at 1:43 PM, Alex Kost <alez...@gmail.com> wrote: > Claes Wallin (韋嘉誠) (2015-06-19 21:51 +0300) wrote:
> I think there is no need in this ↑ line. The commit message should be: > > --8<---------------cut here---------------start------------->8--- > gnu: Add pv. > > * gnu/package/pv.scm (pv): New file. > * gnu-system.am (GNU_SYSTEM_MODULES): Add it. > --8<---------------cut here---------------end--------------->8--- Ok! > And you also need to add "gnu/package/pv.scm" to "gnu-system.am". See > commit 741115b for example. I will take a look. >> +;;; Copyright 2012, 2013, 2015 Ludovic Courts <l...@gnu.org> >> +;;; Copyright 2015 Claes Wallin <claes.wal...@greatsinodevelopment.com> > > IIUC you are the only author of this file, right? Then there is no > place for Ludovic there. Ok, I copied it from another file and modified it, but if the convention is that the boilerplate is too thin to be credited, I'll remove him. >> + #:export (pv)) >> + >> +(define pv > > I think we prefer 'define-public' over exporting the package variables, > but it is probably not a strong convention. define-public is less redundant. I like it. > I would put "Pipe Viewer" in parentheses: > > "pv (Pipe Viewer) is a terminal-based tool for monitoring the progress > > but I think you may ignore this comment. No, it's a fair point. >> +of data through a pipeline. It can be inserted into any normal pipeline > of data through a pipeline. It can be inserted into any normal pipeline > > I realize that you took this description from the home page, but our > convention is to use two spaces between sentences. If there's a convention I'll follow it. I'll just note here once that I disagree with it. Double space is a type-writer convention that "nobody" follows any more. ;-) Regarding scraping the text I tried quickly to see what license the web site text is under, but didn't find anything conclusive. I figured if Debian is copying it, I should be fine. Maybe I'll get back to checking it later. Thanks for the feedback. I have several more packages in the pipeline once I get this one right! -- /c
