Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 9 Nov 2013 02:08:32 +0400
From:      Veniamin Gvozdikov <vg@freebsd.org>
To:        Alexey Dokuchaev <danfe@FreeBSD.org>
Cc:        svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org
Subject:   Re: svn commit: r332677 - in head/sysutils: . dunst
Message-ID:  <A77E62B9-5EC5-4FE3-AE7A-372BCA85EAED@freebsd.org>
In-Reply-To: <20131106074425.GH60770@FreeBSD.org>
References:  <201311040941.rA49fEMj003374@svn.freebsd.org> <20131106074425.GH60770@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
I=92ve just done all recommendations.

Thank you.

On Nov 6, 2013, at 11:44 AM, Alexey Dokuchaev <danfe@FreeBSD.org> wrote:

> On Mon, Nov 04, 2013 at 09:41:14AM +0000, Veniamin Gvozdikov wrote:
>> New Revision: 332677
>> URL: http://svnweb.freebsd.org/changeset/ports/332677
>>=20
>> +CATEGORIES=3D	sysutils
>> +MASTER_SITES=3D	http://www.knopwob.org/public/dunst-release/
>> +
>> +MAINTAINER=3D	rodperson@rodperson.com
>> +COMMENT=3D	Lightweight notification deamon
>=20
> s/deamon/daemon/ perhaps?  You could've added LICENSE=3DBSD, by the =
way.
>=20
>> +LIB_DEPENDS=3D	cairo:${PORTSDIR}/graphics/cairo \
>> +		dbus:${PORTSDIR}/devel/dbus \
>> +		execinfo:${PORTSDIR}/devel/libexecinfo \
>> +		freetype:${PORTSDIR}/print/freetype2 \
>> +		notify:${PORTSDIR}/devel/libnotify \
>> +		pango:${PORTSDIR}/x11-toolkits/pango \
>> +		xdg-basedir:${PORTSDIR}/x11/libxdg-basedir
>=20
> You could've also used modern syntax for LIB_DEPENDS.
>=20
>> +MAKE_ARGS+=3D	MANPREFIX=3D"${PREFIX}/man"
>> +
>> +ALL_TARGET=3D	dunst dunstify dunst.1
>> +INSTALL_TARGET=3D	install
>> +
>> +LDFLAGS+=3D	-O3 -g -Wall -rdynamic -lexecinfo
>=20
> -O3 -g?  Really?  We usually try hard to get rid of this upstream =
shit, not
> to add it.  In rare cases when something obnoxious like this is =
actually
> *required* for software's proper operation, it should be accompanied =
by the
> explanatory comment.
>=20
>> +CFLAGS+=3D	-I${LOCALBASE}/include
>=20
> Polluting CFLAGS with preprocessor option should only be done if =
software
> does not support CPPFLAGS, I hope it was verified that it indeed does =
not.
>=20
>> @@ -0,0 +1,6 @@
>> +Dunst is a lightweight replacement for the notification-daemons =
provided
>> +by most desktop environments. It's very customizable, doesn't depend =
on
>> +any toolkits and therefore fits in those windowmanager centric =
setups we
>> +all love to customize to perfection.
>> +
>> +WWW: http://www.knopwob.org/dunst
>=20
> Nicely written description.  Using double space after the full stop =
would
> make it even better at the right side.  ;-)  I would rather not =
discuss
> the necessity of comma before "and" and if "window manager" ought to =
be
> spelled separately, but would mention that terminating WWW line (per =
PH,
> section 3.2.1) would be nice.
>=20
> ./danfe




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?A77E62B9-5EC5-4FE3-AE7A-372BCA85EAED>