Sender: =?UTF-8?Q?Timo_Ter=C3=A4s?= <timo.teras@gmail.com>
Date: Thu, 21 Jan 2016 11:40:07 +0200
From: Timo Teras <timo.teras@iki.fi>
To: Isaac Dunham <ibid.ag@gmail.com>
Cc: Corentin Henry <corentinhenry@gmail.com>,
 alpine-aports@lists.alpinelinux.org
Subject: Re: [alpine-aports]
Message-ID: <20160121114007.29e8f8e6@vostro>
In-Reply-To: <20160119152853.GA11978@newbook>
References: <1453204241-6685-1-git-send-email-corentinhenry@gmail.com>
	<20160119152853.GA11978@newbook>
Precedence: list
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit

On Tue, 19 Jan 2016 07:28:54 -0800
Isaac Dunham <ibid.ag@gmail.com> wrote:

> On Tue, Jan 19, 2016 at 11:50:37AM +0000, Corentin Henry wrote:
> > hi,
> > 
> > hping3 is a network tool able to send custom TCP/IP packets and to
> > display target replies like ping do with ICMP replies. hping3 can
> > handle fragmentation, and almost arbitrary packet size and content,
> > using the command line interface.
> > 
> > this package builds hping with tcl scripting support.
> > 
> > let me know if there is anything to change, I'm not used yet to
> > packaging.  
> 
> Looks like a typo in package() (hpin3 should presumably be hping3).
> 
> Quote $subpkgdir and $pkgdir ("$subpkgdir").
> 
> Send it all as one patch.
> New APKBUILDS should be buildable from the start; creating a series of
> patches like this:
> 1/4: add APKBUILD (references 1-foo.patch, 2-bar.patch, 3-baz.patch)
> 2/4: add 1-foo.patch
> 3/4: add 2-bar.patch
> 4/4: add 3-baz.patch
> 
> is needlessly cluttered, and likely to break autobuild scripts.
> If you refer to a local source file, you should add it, the checksums,
> and the reference in one commit.

Yes, please flatten it all to one commit.

Additionally, you can look how to do github commitid based packaging.
See. e.g. main/bmd-tools for this:
http://git.alpinelinux.org/cgit/aports/tree/main/bmd-tools/APKBUILD

I believe the compression of manpages happens now automatic, so the
explicit gzip should not be needed in doc(). Well, you'd probably need
to install the .doc files before calling default_doc.

There was also few whitespace mismatches (tabs vs. spaces).

But other than these nitpicks on formatting, it looks good.

Please resubmit with fixes!

Thanks,
Timo



---
Unsubscribe:  alpine-aports+unsubscribe@lists.alpinelinux.org
Help:         alpine-aports+help@lists.alpinelinux.org
---