Hello Duncan,
Welcome to the Alpine Linux community, and thank you for your
contribution. I have placed some feedback inline.
Besides that, aport uses a specific standard for commit messages:
1) A generally followed commit standard where you have a short commit
subject soft limitted to about 50 characters, followed by a blank line,
and then more details about the commit (focussing more on why than what.
See this [post][commit-message] for more information.
2) Commit messages for packages are prefixed with the repo and package
name like this: 'testing/dcc: '.
So the commit message for this patch would be:
```
testing/dcc: new aport
```
The fact that there are subpackages is not that interesting (most
packages will have subpackages).
I hope this is clear, otherwise, feel free to ask questions.
Kind regards, Kevin.
On Fri, Jan 24, 2020 at 09:46:54PM +0000, Duncan Bellamy wrote:
> ---> testing/dcc/APKBUILD | 61 ++++++++++++++++++++++++++++++++++++++++++++> 1 file changed, 61 insertions(+)> create mode 100644 testing/dcc/APKBUILD> > diff --git a/testing/dcc/APKBUILD b/testing/dcc/APKBUILD> new file mode 100644> index 0000000000..6efcfd6e3b> --- /dev/null> +++ b/testing/dcc/APKBUILD> @@ -0,0 +1,61 @@> +# Contributor: Duncan Bellamy <dunk@denkimushi.com>> +# Maintainer: Duncan Bellamy <dunk@denkimushi.com>> +pkgname="dcc"> +pkgver="2.3.167"> +pkgrel=1> +pkgdesc="Distributed Checksum Clearinghouses or DCC spam filter"> +url="https://www.dcc-servers.net/dcc/"> +arch="all"> +license="Distributed Checksum Clearinghouse"
If the license is not on the [spdx][spdx] list, this should be `custom`.
> +pkgusers="_dcc"> +pkggroups="_dcc"
Any reason why these are prefixed with an '_'? In any case, this just
makes sure that they exist during building. You still need to add a
pre-install script to make sure they exist on the target system.
> +depends="wget"> +makedepends="libmilter-dev"> +subpackages="$pkgname-dccifd $pkgname-dccm $pkgname-doc"> +source="https://www.dcc-servers.net/src/dcc/old/dcc-2.3.167.tar.Z"> +options="!check"> +> +prepare() {> + default_prepare
Inconsistent indentation here
> + sed -i 's+DCC_UNIX+DCC_UNIX\n#include <sys\/types.h>+g' "$srcdir"/$pkgname-$pkgver/include/dcc_types.h
Dependending how much is changed, we prefer using patches instead of
`sed -i`.
> +}> +> +unpack() {
Please call default_unpack here as well, as it also includes some other
functionality that is missing now.
Another option is to specify the archive name when downloading in the
source variable:
```sh
source="$pkgname-$pkgver.tar.gz::https://www.dcc-servers.net/src/dcc/old/dcc-2.3.167.tar.Z"
```
Then the default behaviour should suffice.
> + cd "$srcdir"> + tar -xzf dcc*.tar.Z> +}> +> +build() {> + ./configure \> + --with-installroot="$pkgdir" \> + --bindir="/usr/bin" \> + --mandir="/usr/man" \> + --with-uid=_dcc> + make> +}> +> +package() {> + make install> + install -Dm644 "$builddir"/LICENSE \> + "$pkgdir"/usr/share/licenses/$pkgname/LICENSE> +> + chmod 755 "$pkgdir"/var/dcc/libexec/dccsight> + cd "$pkgdir"/usr/bin> + chmod 755 cdcc dccproc> +}> +> +dccifd() {> + depends="dcc"> + mkdir -p "$subpkgdir"/var/dcc/libexec> + cd "$pkgdir"/var/dcc/libexec> + mv dccifd start-dccifd "$subpkgdir"/var/dcc/libexec/
abuild now has a function called `amove` that makes this easier. The
above 3 lines can be replaced with:
```sh
amove var/dcc/libexec/dccifd var/dcc/libexec/start-dccifd
```
> +}> +> +dccm() {> + depends="dcc"> + mkdir -p "$subpkgdir"/var/dcc/libexec> + cd "$pkgdir"/var/dcc/libexec> + mv dccm start-dccm "$subpkgdir"/var/dcc/libexec/
`amove` can be used here as well.
> +}> +> +sha512sums="384a572e5b18bed6aed08dce6ebc468d5737b0cb4774fe502f527b101a38b4bec1fdd73384c6fb437c21ae46aa56ae04c5c459737cdda6ab3ce186ff4f77cf98 dcc-2.3.167.tar.Z"> -- > 2.24.1
[commit-message]:https://chris.beams.io/posts/git-commit/
[spdx]:https://spdx.org/licenses/
Hi Kevin,
Thanks for the feedback, I will edit and resubmit.
I submitted an rspamd patch as well to update to 2.2 and tried to amend the commit after I found some typos but it kept submitting the old patch with new subject.
Thanks,
Duncan
> On 25 Jan 2020, at 15:28, Kevin Daudt <kdaudt@alpinelinux.org> wrote:> > Hello Duncan,> > Welcome to the Alpine Linux community, and thank you for your> contribution. I have placed some feedback inline.> > Besides that, aport uses a specific standard for commit messages:> > 1) A generally followed commit standard where you have a short commit> subject soft limitted to about 50 characters, followed by a blank line,> and then more details about the commit (focussing more on why than what.> See this [post][commit-message] for more information.> 2) Commit messages for packages are prefixed with the repo and package> name like this: 'testing/dcc: '.> > So the commit message for this patch would be:> > ```> testing/dcc: new aport> ```> > The fact that there are subpackages is not that interesting (most> packages will have subpackages).> > I hope this is clear, otherwise, feel free to ask questions.> > Kind regards, Kevin.> >> On Fri, Jan 24, 2020 at 09:46:54PM +0000, Duncan Bellamy wrote:>> --->> testing/dcc/APKBUILD | 61 ++++++++++++++++++++++++++++++++++++++++++++>> 1 file changed, 61 insertions(+)>> create mode 100644 testing/dcc/APKBUILD>> >> diff --git a/testing/dcc/APKBUILD b/testing/dcc/APKBUILD>> new file mode 100644>> index 0000000000..6efcfd6e3b>> --- /dev/null>> +++ b/testing/dcc/APKBUILD>> @@ -0,0 +1,61 @@>> +# Contributor: Duncan Bellamy <dunk@denkimushi.com>>> +# Maintainer: Duncan Bellamy <dunk@denkimushi.com>>> +pkgname="dcc">> +pkgver="2.3.167">> +pkgrel=1>> +pkgdesc="Distributed Checksum Clearinghouses or DCC spam filter">> +url="https://www.dcc-servers.net/dcc/">> +arch="all">> +license="Distributed Checksum Clearinghouse"> > If the license is not on the [spdx][spdx] list, this should be `custom`.> >> +pkgusers="_dcc">> +pkggroups="_dcc"> > Any reason why these are prefixed with an '_'? In any case, this just> makes sure that they exist during building. You still need to add a> pre-install script to make sure they exist on the target system.> >> +depends="wget">> +makedepends="libmilter-dev">> +subpackages="$pkgname-dccifd $pkgname-dccm $pkgname-doc">> +source="https://www.dcc-servers.net/src/dcc/old/dcc-2.3.167.tar.Z">> +options="!check">> +>> +prepare() {>> + default_prepare> > Inconsistent indentation here> >> + sed -i 's+DCC_UNIX+DCC_UNIX\n#include <sys\/types.h>+g' "$srcdir"/$pkgname-$pkgver/include/dcc_types.h> > Dependending how much is changed, we prefer using patches instead of> `sed -i`.> >> +}>> +>> +unpack() {> > Please call default_unpack here as well, as it also includes some other> functionality that is missing now.> > Another option is to specify the archive name when downloading in the> source variable:> > ```sh> source="$pkgname-$pkgver.tar.gz::https://www.dcc-servers.net/src/dcc/old/dcc-2.3.167.tar.Z"> ```> > Then the default behaviour should suffice.> >> + cd "$srcdir">> + tar -xzf dcc*.tar.Z>> +}>> +>> +build() {>> + ./configure \>> + --with-installroot="$pkgdir" \>> + --bindir="/usr/bin" \>> + --mandir="/usr/man" \>> + --with-uid=_dcc>> + make>> +}>> +>> +package() {>> + make install>> + install -Dm644 "$builddir"/LICENSE \>> + "$pkgdir"/usr/share/licenses/$pkgname/LICENSE>> +>> + chmod 755 "$pkgdir"/var/dcc/libexec/dccsight>> + cd "$pkgdir"/usr/bin>> + chmod 755 cdcc dccproc>> +}>> +>> +dccifd() {>> + depends="dcc">> + mkdir -p "$subpkgdir"/var/dcc/libexec>> + cd "$pkgdir"/var/dcc/libexec>> + mv dccifd start-dccifd "$subpkgdir"/var/dcc/libexec/> > abuild now has a function called `amove` that makes this easier. The> above 3 lines can be replaced with:> > ```sh> amove var/dcc/libexec/dccifd var/dcc/libexec/start-dccifd> ```> >> +}>> +>> +dccm() {>> + depends="dcc">> + mkdir -p "$subpkgdir"/var/dcc/libexec>> + cd "$pkgdir"/var/dcc/libexec>> + mv dccm start-dccm "$subpkgdir"/var/dcc/libexec/> > `amove` can be used here as well.> >> +}>> +>> +sha512sums="384a572e5b18bed6aed08dce6ebc468d5737b0cb4774fe502f527b101a38b4bec1fdd73384c6fb437c21ae46aa56ae04c5c459737cdda6ab3ce186ff4f77cf98 dcc-2.3.167.tar.Z">> -- >> 2.24.1> > [commit-message]:https://chris.beams.io/posts/git-commit/> [spdx]:https://spdx.org/licenses/