Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Nov 2015 19:15:46 -0800
From:      Bryan Drewery <bdrewery@FreeBSD.org>
To:        Alexey Dokuchaev <danfe@FreeBSD.org>
Cc:        ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org
Subject:   Re: svn commit: r401299 - head/security/openssh-portable/files
Message-ID:  <56440462.6000803@FreeBSD.org>
In-Reply-To: <20151112030538.GA71430@FreeBSD.org>
References:  <201511112121.tABLLjO6051679@repo.freebsd.org> <20151112021225.GB43902@FreeBSD.org> <5643FC04.4020001@FreeBSD.org> <20151112030538.GA71430@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On 11/11/15 7:05 PM, Alexey Dokuchaev wrote:
> On Wed, Nov 11, 2015 at 06:40:04PM -0800, Bryan Drewery wrote:
>> On 11/11/15 6:12 PM, Alexey Dokuchaev wrote:
>>> On Wed, Nov 11, 2015 at 09:21:45PM +0000, Bryan Drewery wrote:
>>>> New Revision: 401299
>>>> URL: https://svnweb.freebsd.org/changeset/ports/401299
>>>>
>>>> Log:
>>>>   Make portlint stop spamming me.  It's gotten quite silly.
>>>
>>> [...]
>>> As John had said on IRC, this helps to get consistent patches, becaus=
e
>>> prople rarely think about these little details ("repo churn? who care=
s
>>> about it") and portlint(1) warning gives them simple and straight cou=
rse
>>> of action.  Yet it's true that the check could probably be made somew=
hat
>>> smarter than simple grepping for "UTC".
>>>
>>> TL;DR: instead of adding noise to the patches, it's better to improve
>>> portlint(1).  Or learn how to ignore its warnings. ;-)
>>
>> We should just ignore portlint at our own discretion since it grows
>> stupid warnings like this?
>=20
> Ignoring stupid warnings was listed second after improving portlint(1).
> But yes, portlint(1) can be wrong, so, answering your question, yes, "w=
e
> should just ignore portlint at our own discretion".  Every software has
> bugs, checkers can give false positives.
>=20
>> Mission accomplished?
>=20
> Mission is not to follow *everything* any lint tool tells you about you=
r
> code and stuff.  The mission is to have that stuff working and neat, wi=
th
> some tools' help or without.
>=20
> That's why I think that adding noise to patches is wrong approach: it m=
akes
> the stuff (patches) less neat and thus portlint(1) warnings more import=
ant,
> while it should be the other way around.
>=20

If you have to tell people to ignore a warning, the warning should come
OUT or be changed.

Conditioning people to ignore warnings whenever they feel like it, or
because the warnings are often false-positive, is not productive. It's
why I spent so much time making check-plist correct last year in r351587.

The problem here is not "repo churn", it is creating busy work for
people. It is unfortunate that portlint is growing stuff like this and
the actually wrong advice of sorting Uses, since it is just creating
work for people where work is not needed. There's probably < 3 people
who care about these "consistency" issues. We should not push back on
contributors because they missed an optional / or did not generate their
patch with -p or started the comment with an 'A' (where upstream may
even have it). It's counter-productive.

This check considers patches with header comments to be wrong. That's
not right and I'm sure it was just overlooked. We should be doing the
opposite though, encouraging comments in patches as to why they are
there and their upstream status, rather than telling people to blindly
blow away useful information.

I do think it is worth having -p generated diffs, but 'makepatch' did
not do that until relatively recently. So this warning will appear to be
false-positive to people who know they did use 'makepatch' in the past.

However, I don't agree with the warning since it's really asking people
to do your work and lacks the larger vision of things like upstream
status and whatever else I cannot think of (WE NEED MORE VISION IN
PORTS). Looking at the original PR I see no evidence that this warning
was added to catch actual bad patches, but only to encourage people to
generate them in the new format. I keep beating this drum, let's not
make people do busy work unless there's a really good reason. New PLIST
format? Great, but make it worth doing, not just for the sake of it
looking prettier, make it happen with sub-packages. There's 24k ports,
we need things like provides/requires, not whitespace consistency
distractions.

--=20
Regards,
Bryan Drewery



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