From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id ED30C2117CE94 for ; Tue, 6 Nov 2018 07:24:33 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 813B33082143; Tue, 6 Nov 2018 15:24:33 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-95.rdu2.redhat.com [10.10.120.95]) by smtp.corp.redhat.com (Postfix) with ESMTP id 34845194AE; Tue, 6 Nov 2018 15:24:31 +0000 (UTC) To: Ard Biesheuvel , Fu Siyuan Cc: "edk2-devel@lists.01.org" , Julien Grall References: <20181106012403.84760-1-siyuan.fu@intel.com> <20181106012403.84760-2-siyuan.fu@intel.com> From: Laszlo Ersek Message-ID: <7ab7d41e-ec48-3fb2-28a7-83c7471d7d0e@redhat.com> Date: Tue, 6 Nov 2018 16:24:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Tue, 06 Nov 2018 15:24:33 +0000 (UTC) Subject: Re: [PATCH v3 1/1] ArmVirtPkg: Replace obsoleted network drivers from platform DSC/FDF. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Nov 2018 15:24:34 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/06/18 13:32, Ard Biesheuvel wrote: > On 6 November 2018 at 02:24, Fu Siyuan wrote: >> V3: >> Remove duplicate library added in v2 patch, since ArmVirtPkg.dsc.inc >> already have them. Just remove the if...end there is enough. >> >> V2: >> Add missing library instance for NetworkPkg iSCSI driver. >> > > Please don't put the patch revision history in the commit log. Put it > below the --- > >> This patch replaces the MdeModulePkg TCP, PXE and iSCSI driver with those >> ones in NetworkPkg. These 3 drivers in MdeModulePkg are not being actively >> maintained and will be removed from edk2 master soon. >> >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Cc: Julien Grall >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Fu Siyuan >> --- > > ... here ... > > The patch looks fine to me > > Reviewed-by: Ard Biesheuvel > > but please don't merge it until after the next stable tag has been created This is not a bad idea (see also your discussion with Leif); however it does create a bit of inconsistency with how the other platform DSC/FDF files have been handled. (The changes have been pushed for those.) Again, I don't disagree, and I don't mind if ArmVirt is handled differently. It's just that we should have handled this more uniformly, I believe. In retrospect, I would have also appreciated if the patches had referenced , even though they only implement "prep" work for now, on the platform DSC/FDF level, and not the actual driver removal. For example, the important explanation about MdeModulePkg's iSCSI driver implementing its own MD5 algo cannot be connected to the OVMF commit now (d2f1f6423bd1). I have copied the most relevant passage from the cover letter of this series into TianoCore BZ#1278, but the commit in question doesn't reference any BZ, so the link cannot be established. Thanks Laszlo