Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 10 Feb 2021 02:35:01 +0000
From:      Alexey Dokuchaev <danfe@freebsd.org>
To:        Joseph Mingrone <jrm@freebsd.org>
Cc:        ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: svn commit: r564803 - in head/sysutils: . rmlint rmlint/files
Message-ID:  <20210210023501.GA65146@FreeBSD.org>
In-Reply-To: <202102091926.119JQQL8050604@repo.freebsd.org>
References:  <202102091926.119JQQL8050604@repo.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Feb 09, 2021 at 07:26:26PM +0000, Joseph Mingrone wrote:
> New Revision: 564803
> URL: https://svnweb.freebsd.org/changeset/ports/564803
> 
> Log:
>   New port, sysutils/rmlint: Remove duplicates from your filesystem

Hi Joseph,

I'm afraid this was not sufficiently reviewed, there are number of
issues with this port, I'll comment on the more serious ones.

> ...
> +BUILD_DEPENDS=	${LOCALBASE}/lib/libglib-2.0.so:devel/glib20 \

This line looks quite weird, especially given USE_GNOME=glib20 below.
What exactly it's supposed to achieve?

> +		gettext:devel/gettext \
> +		pkgconf:devel/pkgconf \

We have USES for these.

> +USES=		gnome python scons

If build, run, and test arguments for "python" are omitted, it will
be added as BUILD_DEPENDS, RUN_DEPENDS and TEST_DEPENDS, now let's
see: scons already implies python:build, and the program is written
in C.  It does uses Python for optional GUI, but it's explicitly
disabled with --without-gui passed on the MAKE_ARGS.  I didn't look
if there are any tests which require it, but given the amount of
mistakes with this port, it's probably also wrong.

> +USE_GITHUB=	yes
> +GH_ACCOUNT=	sahib
> +GH_PROJECT=	rmlint

This is the default as it matches the port name.

> +OPTIONS_DEFINE=	NLS
> +
> +NLS_USES=		gettext-runtime
> +NLS_CONFIGURE_WITH=	gettext

If gettext is optional, why was it put on global BUILD_DEPENDS earlier?

> +@@ -316,6 +316,8 @@ int rm_xattr_clear_hash(RmFile *file, RmSession *sessi
> + 
> + #if HAVE_XATTR
> + 
> ++#if HAVE_XATTR
> ++

Two consecutive "#if HAVE_XATTR" lines look strange.  Could the existing
one be used?

> +++ head/sysutils/rmlint/pkg-descr
> @@ -0,0 +1,5 @@
> +rmlint is an "extremely fast tool to remove duplicates and other lint
> +from your filesystem."

This description essentially repeats the COMMENT and thus is not really
helpful.  The one provided on the website is longer and better (starts
with "rmlint finds space waste and other broken things on your filesystem
and offers to remove it.  It is able to find: ..."

> +
> +WWW: https://rmlint.rtfd.org/

One WWW line is enough (see PHB Section 3.2.1).  Also, we typically
would chase HTTP redirects (302 in this case).

> +WWW: https://github.com/sahib/rmlint

./danfe



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20210210023501.GA65146>