Date: Fri, 24 May 2013 03:27:11 +0000 From: Alexey Dokuchaev <danfe@FreeBSD.org> To: Jason Helfman <jgh@FreeBSD.org> Cc: svn-ports-head@freebsd.org, svn-ports-all@freebsd.org, ports-committers@freebsd.org Subject: Re: svn commit: r318883 - in head/net-im: ejabberd iserverd jabber-pyaim jggtrans naim py-xmpppy-yahoo sim-im-devel skype skype-devel tkabber-devel Message-ID: <20130524032711.GB48975@FreeBSD.org> In-Reply-To: <201305231610.r4NGAhke073280@svn.freebsd.org> References: <201305231610.r4NGAhke073280@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, May 23, 2013 at 04:10:43PM +0000, Jason Helfman wrote: > New Revision: 318883 > URL: http://svnweb.freebsd.org/changeset/ports/318883 > > -.if defined(NOPORTDOCS) > +.if ! ${PORT_OPTIONS:MDOCS} > MAKE_ARGS+= NOPORTDOCS=${NOPORTDOCS} > .endif This is a bit worrisome: it seems that this code still depends on NOPORTDOCS variable (while this commit supposedly gets rid of it). Could this become a problem in the future? > -OPTIONS= KQUEUE "Use kqueue(2) instead of poll(2)" on \ > - DEBUG "Enable debugging symbols" off > +OPTIONS_DEFINE= KQUEUE DEBUG > +KQUEUE_DESC= kqueue(2) instead of poll(2) I think you've missed "Use" word here. Now options description both reads (missing verb or subject word like "support"; kqueue/poll here are more of objects in grammatical sense) and looks bad (starts with lowercase letter). > -OPTIONS= EJABBERD "Use transport with ejabberd" off \ > - TWISTED1 "Use old py-twisted 1.x" off > +OPTIONS_DEFINE= EJABBERD TWISTED1 > +EJABBERD_DESC= transport with ejabberd > +TWISTED1_DESC= old py-twisted 1.x Ditto. > +.if ${PORT_OPTIONS:MDOCS} > ${MKDIR} ${DOCSDIR} > .for portdoc in ${PORTDOCS} > ${INSTALL_DATA} ${WRKSRC}/${portdoc} ${DOCSDIR}/ There is a nice (concise yet readable) way of avoiding .for loop here: .if ${PORT_OPTIONS:MDOCS} @${MKDIR} ${DOCSDIR} ${INSTALL_DATA} ${PORTDOCS:S,^,${WRKSRC}/,} ${DOCSDIR} .endif > -OPTIONS= NODEBUG "Turn off debugging code" off \ > - EJABBERD "Use transport with ejabberd" off > +OPTIONS_DEFINE= DEBUG EJABBERD > +EJABBERD_DESC= transport with ejabberd Ditto. > -OPTIONS= DETACH "Enable 'detach' feature (requires misc/screen)" off > +OPTIONS_DEFINE= DETACH > +DETACH_DESC= Activate 'detach' feature (requires misc/screen) "Enable" is not a taboo :) and suits just fine here. It is normally dropped when used together with "support" as being redundant: "Blah support" already implies (at least in options dialog context) that it can be enabled (checked) or disabled (unchecked). "Activate" is less neutral; it might confuse users (it does confuse me): I think that activating 'detach' feature is what happens during runtime when user is actually *detaches*, not when this feature is *enabled* during the port config/build time. > -OPTIONS= EJABBERD "Use transport with ejabberd" off > +OPTIONS_DEFINE= EJABBERD > +EJABBERD_DESC= transport with ejabberd This one is a good example of why weak-subject descriptions look bad. Not to mention initial lowercase letter... > +OPTIONS_DEFINE= VIDEO NVIDIA_GL > +VIDEO_DESC=[broken] Video support > +NVIDIA_GL_DESC= libGL provided by NVidia binary drivers Ditto; also it seems indentation in VIDEO_DESC= line got broken. > -OPTIONS= VIDEO "Video sending support via multimedia/webcamd" on \ > - NVIDIA_GL "Use libGL provided by NVidia binary drivers" off > + > +OPTIONS_DEFINE= VIDEO NVIDIA_GL > +NVIDIA_GL_DESC= libGL provided by NVidia binary drivers Ditto. They read better with "Use". ./danfe
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130524032711.GB48975>