From owner-svn-ports-head@freebsd.org Thu Nov 12 07:40:35 2015 Return-Path: Delivered-To: svn-ports-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 49293A2D72E; Thu, 12 Nov 2015 07:40:35 +0000 (UTC) (envelope-from freebsd.contact@marino.st) Received: from shepard.synsport.net (mail.synsport.com [208.69.230.148]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 2055F14DC; Thu, 12 Nov 2015 07:40:34 +0000 (UTC) (envelope-from freebsd.contact@marino.st) Received: from [192.168.1.22] (210.Red-81-38-187.dynamicIP.rima-tde.net [81.38.187.210]) (using TLSv1 with cipher DHE-RSA-AES128-SHA (128/128 bits)) (No client certificate requested) by shepard.synsport.net (Postfix) with ESMTP id 6EB2543BCF; Thu, 12 Nov 2015 01:40:31 -0600 (CST) Subject: Re: svn commit: r401299 - head/security/openssh-portable/files To: Bryan Drewery , Alexey Dokuchaev References: <201511112121.tABLLjO6051679@repo.freebsd.org> <20151112021225.GB43902@FreeBSD.org> <5643FC04.4020001@FreeBSD.org> <20151112030538.GA71430@FreeBSD.org> <56440462.6000803@FreeBSD.org> Cc: ports-committers@freebsd.org, svn-ports-all@freebsd.org, svn-ports-head@freebsd.org From: John Marino X-Enigmail-Draft-Status: N1110 Reply-To: marino@freebsd.org Message-ID: <5644426B.7000908@marino.st> Date: Thu, 12 Nov 2015 08:40:27 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Thunderbird/38.0.1 MIME-Version: 1.0 In-Reply-To: <56440462.6000803@FreeBSD.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-BeenThere: svn-ports-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the ports tree for head List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Nov 2015 07:40:35 -0000 On 11/12/2015 4:15 AM, Bryan Drewery wrote: > If you have to tell people to ignore a warning, the warning should come > OUT or be changed. To be fair, we'd prefer that the person heed the warning and regenerate the patch. However, some people rather spend 30 minutes complaining that they shouldn't have to than 2 minutes regenerating the patches. We say "ignore it / grep it out" to folks that take the above stance because it's not worth another discussion. > 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. Portlint is not gospel. There are plenty of emitted warnings that on stuff that is okay. > 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, Re: sorting: I'd argue that USES tools that clobber each other to redefine tool names are wrong. Especially if multiple versions of the same tool are need for the port. If we need fmake and gmake and/or regular make then probably the port should be patched to use a single tool (for example) > 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. There is a distinction between "not needed" and "required"/"desired". Regenerating patches is not required, but it is desired if they port is being worked on substantially anyway. > 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. This is spreading misinformation. It checks the header to determine if the patch is old or home-grown. It doesn't have to every single missing aspect. If the "UTC" is missing, there's a very high probabibilty that the patch is not in the current makepatch format. Re: comments in patches. You can get carried away. You know how danfe is on consistency? There are people in pkgsrc that rip you every time you submit a patch w/o a comment, even the change is obvious and self explanatory. No thank you. the takeaway: "make makepatch" needs to be improved to do two things 1) conserve existing comments during regeneration 2) If an existing patch touches one or more files, the regenerated version needs to cover the same files as the patch it replaces. > 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. This is exaggerated. Even if they were so confused, it would last exactly one port when "svn diff" shows the difference. > 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). "your work"? It's not "his" work, and w/o people, people revert good work of others. This is a fact, this is why the warning appeared in portlint because it was happening A LOT. > 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. You make it sound like it doesn't take 20 seconds to 2 minutes to do it on top of a port overhaul they are doing anyway. > 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. This is an entirely different topic, one that you misrepresented the first time and news flash: There is no such thing as "subpackages" right now. Any work on that can *EASILY* adjust. we're talking minutes to improve on existing PROTOYPE code. Let's not ambush this thread though. John