svn commit: r332677 - in head/sysutils: . dunst

[Veniamin Gvozdikov]

I’ve just done all recommendations.

Thank you.

On Nov 6, 2013, at 11:44 AM, Alexey Dokuchaev <danfe at 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
>> 
>> +CATEGORIES=	sysutils
>> +MASTER_SITES=	http://www.knopwob.org/public/dunst-release/
>> +
>> +MAINTAINER=	rodperson at rodperson.com
>> +COMMENT=	Lightweight notification deamon
> 
> s/deamon/daemon/ perhaps?  You could've added LICENSE=BSD, by the way.
> 
>> +LIB_DEPENDS=	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
> 
> You could've also used modern syntax for LIB_DEPENDS.
> 
>> +MAKE_ARGS+=	MANPREFIX="${PREFIX}/man"
>> +
>> +ALL_TARGET=	dunst dunstify dunst.1
>> +INSTALL_TARGET=	install
>> +
>> +LDFLAGS+=	-O3 -g -Wall -rdynamic -lexecinfo
> 
> -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.
> 
>> +CFLAGS+=	-I${LOCALBASE}/include
> 
> 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.
> 
>> @@ -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
> 
> 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.
> 
> ./danfe