From patchwork Thu Apr 12 18:13:59 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Leif Lindholm X-Patchwork-Id: 133305 Delivered-To: patch@linaro.org Received: by 10.46.84.29 with SMTP id i29csp1946862ljb; Thu, 12 Apr 2018 11:15:06 -0700 (PDT) X-Google-Smtp-Source: AIpwx4/+2UiJe8+4VoOBIF47jSNrJlvGIKPNSlTZbxtABYJeQa0azm+5sGuGu35A5rRFRQeYBiQQ X-Received: by 2002:a17:902:5a42:: with SMTP id f2-v6mr2097163plm.85.1523556906334; Thu, 12 Apr 2018 11:15:06 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1523556906; cv=none; d=google.com; s=arc-20160816; b=BLHqEb5mIoqISWM3C5N61Gj8KfggUhwX/R2GvYquEWS3pAlMD7dzrlcg+qWM/SEOfn HIK//5yL+T+1pT78TbenpzViLF28r4U88bb/nK1+A/539JT/wv0Nu68w8pyR/fMuyKEF PBExwzOpuuxNcN40QWWdWYOT1dW51SQq1CZBZpmj7drWqTTVgpRzgUc4XZTIxp8z08xe TbKTWdEdulp5mLAJ7frAFzYi7emYUDJLrZ4rJ0Cy3Q/3+N9SwKfYGjkeLIinPOpPX/s1 zSouKmDOqjng2pt1VuhvDQUIG80xI0UTujOODItCukou4sgqo3A9on874wJ443rr7Gio Zg6g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:cc:list-subscribe :list-help:list-post:list-archive:list-unsubscribe:list-id :precedence:subject:user-agent:in-reply-to:content-disposition :mime-version:references:message-id:to:from:date:dkim-signature :delivered-to:arc-authentication-results; bh=PIWChEUizvbPaWe42w1xgBZYWhBOAXFnk+PCyIR2CwE=; b=fsuoaeFyCzqV264a8Dt0sx6nZmNCbmEJ7u/qf5hCBMkNaeG3Zam5VtDLh6hAOyr2UH z8DV/SfUeGnNW7JaLMd5EwkwRoIzGisczzgR/PR0Yuc9zCdr0Dcpc4iNqlCDcO1mOkdE 6JR3bzP6C0vvd0DwqwyUfIBfpZK6LkAJ5AenLW+MzvxyYmZCDTIArseVIQrgJ/jioYje UseePnI0NHn0IiCBE2AlruwXMjvcDknSnkBPUs1FrpTIVxqtiCDS1r1FaOTbJqCbKgqz TGMw2kjutqELFmirJICjoVvsFuPW/MCFCm2t7bEQgBC2O97F04BE9rrQqoxGV0Xi4bUb +0ow== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=hdTSmS0/; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from ml01.01.org (ml01.01.org. [2001:19d0:306:5::1]) by mx.google.com with ESMTPS id i90-v6si3840568pli.717.2018.04.12.11.15.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 12 Apr 2018 11:15:06 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) client-ip=2001:19d0:306:5::1; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=hdTSmS0/; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 9E32D2272963B; Thu, 12 Apr 2018 11:15:05 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=74.125.82.50; helo=mail-wm0-f50.google.com; envelope-from=leif.lindholm@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wm0-f50.google.com (mail-wm0-f50.google.com [74.125.82.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 4A0762272960B for ; Thu, 12 Apr 2018 11:15:04 -0700 (PDT) Received: by mail-wm0-f50.google.com with SMTP id r191so271wmg.4 for ; Thu, 12 Apr 2018 11:15:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ArHJMsim/zv/eGimw4mqxi8Wrk3EDRWu+2Zp50cGl08=; b=hdTSmS0/xgRaPgLeORZDsZB0gUe87m+5zZYoDHSkXYwrmd62NOJFl8B1lMN11mAN20 OD7Mk3OSbSaCKMime4/8yoaB6v7voQkKYmbgxZIU0WEGH6AgYETQpu4lZh4+8Mb0Hyag 4uXXEDjyxaeqGRoZZOspVlgYWe1Q0DlSPJ51I= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ArHJMsim/zv/eGimw4mqxi8Wrk3EDRWu+2Zp50cGl08=; b=g966e7jpuxfv+Pb/cnieltGtgJv4uwVsZNsT+9PoNRPYuwcHdHh4IyPHTsniQG4Mo1 izYSCjn+b2EcDc+F0WlL3eOmDi7PxmlJUqzZk0d4gNGu6BdAuGB/tF5SrMxh6Kbq5QKg fTDiHNqPXHo9zENcZiH6QK0q+3Iq3zu8OcjTq8BDEmLzd7G0F15osrfYEDpdf0TNlv/i iQf1pctJc6YHRRTnT5ytiQCR1WlPQFMOE5DKKZLCcvFgiylBA1MnfVoomOPX9U7H+6R6 uER9ptjhwhnF4gCFAqjUY/0VVMX7eMziSEH+EEBbehavJJgo7eSE3OmTsRxxBqVIJeIx nhbg== X-Gm-Message-State: ALQs6tBUThogghU3DC3APwb9MTgz86tJ5SDOTPAMwfclQDJ/3Vwva1cX pIM8FOhMr4fs7i0XOgte5HR/Q26oPY0= X-Received: by 10.28.234.12 with SMTP id i12mr1434382wmh.161.1523556842355; Thu, 12 Apr 2018 11:14:02 -0700 (PDT) Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id s21sm3281961wra.66.2018.04.12.11.14.00 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 12 Apr 2018 11:14:01 -0700 (PDT) Date: Thu, 12 Apr 2018 19:13:59 +0100 From: Leif Lindholm To: Laszlo Ersek Message-ID: <20180412181359.rvt2s4fhyuxmj465@bivouac.eciton.net> References: <20180412005540.26651-1-lersek@redhat.com> <20180412172318.jtugn7p3w2qgr24y@bivouac.eciton.net> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Subject: [edk2] derailing into patch style discussion X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "edk2-devel@lists.01.org" Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" Since you already have my r-b on the set, I'll pick up the style topic, partly because I'm not sure if I've ever explained my thinking publicly in words that anyone other than Ard understands. On Thu, Apr 12, 2018 at 07:45:19PM +0200, Laszlo Ersek wrote: > > Well, there are a couple of places where I could nitpick on > > alphabetical sorting of includes, > > And, believe me, you would have my total agreement :), but in such > cases, there's always a fork in the road: (a) add a separate patch that > first sorts the includes and [LibraryClasses], without functional > changes, or (b) just stick with the existing disorder, and get in and > out as surgically as possible. My personal preference is (a), but it has > drawn disagreement -- even accusations of pedantry :) :) --, and/or > suggestions to squash such patches with functional changes, in the past, > so I trod more lightly now. Rest assured, I didn't *miss* those places, > I just elected to close my eyes! ;) Right :) So, yes - I do strongly agree with the idea of keeping functionality and style changes separate (ask Evan), so that wasn't exactly what I was referring to. In general my internal "optimal" situation is one where the purely functional diff leaves the code in a (quite subjectively, since it's still not conformant) better state on average than it was. My subjective mark of optimal is that which would minimise any subsequent patch that was _only_ a style cleanup. To pick an example from this set (1/10): + ArmPlatformPkg/ArmPlatformPkg.dec EmbeddedPkg/EmbeddedPkg.dec MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec - ArmPlatformPkg/ArmPlatformPkg.dec instead of [Packages] - MdePkg/MdePkg.dec - MdeModulePkg/MdeModulePkg.dec ArmPlatformPkg/ArmPlatformPkg.dec EmbeddedPkg/EmbeddedPkg.dec + MdePkg/MdePkg.dec + MdeModulePkg/MdeModulePkg.dec Anyway, this is all an aside - I just thought I would give you an insight into the mind of Leif. / Leif _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf index 6deb8c3f675c..61ad89af2758 100644 --- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf +++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf @@ -41,14 +41,14 @@ [LibraryClasses] PrintLib UefiDriverEntryPoint IoLib ArmLib [Protocols] - gHardwareInterruptProtocolGuid - gEfiCpuArchProtocolGuid + gHardwareInterruptProtocolGuid ## PRODUCES + gEfiCpuArchProtocolGuid ## CONSUMES ## NOTIFY --- This one is pretty straightforward - without touching any non-modified lines, these _could_ be reordered alphabetically. (Yes, that change may have functional side-effects, but only on undefined behaviour, so no different from rebasing to a version with newer BaseTools can give you.) Where the minimum diff logic comes in is in situations like (6/10): diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf index 812dafd065b2..c40ac27a6599 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf @@ -28,12 +28,13 @@ [Sources.common] NorFlashBlockIoDxe.c [Packages] MdePkg/MdePkg.dec MdeModulePkg/MdeModulePkg.dec ArmPlatformPkg/ArmPlatformPkg.dec + EmbeddedPkg/EmbeddedPkg.dec --- Applying my "optimal" rule here would have meant inserting the new line before MdePkg/MdeModulePkg .decs instead. That way a cleanup patch would end up doing [Packages]