From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 ADE6521E11D64 for ; Wed, 9 Aug 2017 04:12:48 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 5A629B1E4D; Wed, 9 Aug 2017 11:15:06 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 5A629B1E4D Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-102.phx2.redhat.com [10.3.116.102]) by smtp.corp.redhat.com (Postfix) with ESMTP id EF3FF845DE; Wed, 9 Aug 2017 11:15:04 +0000 (UTC) To: Eric Dong References: <20170809063207.19284-1-eric.dong@intel.com> From: Laszlo Ersek Cc: edk2-devel@lists.01.org, "Jordan Justen (Intel address)" , Michael Kinney , "Ni, Ruiyu" , Jeff Fan Message-ID: <5de2b848-7a40-4326-12c7-abcba8e71183@redhat.com> Date: Wed, 9 Aug 2017 13:15:03 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170809063207.19284-1-eric.dong@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 09 Aug 2017 11:15:06 +0000 (UTC) Subject: Re: [Patch 0/2] Add comments for new Pcds X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Aug 2017 11:12:48 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Eric, On 08/09/17 08:32, Eric Dong wrote: > Add missed Pcds definition and comments in dec and inf files. > > Eric Dong (2): > UefiCpuPkg: Add comments for PCDs definition. > UefiCpuPkg UefiCpupkg.uni: Add new pcds comments. > > UefiCpuPkg/UefiCpuPkg.dec | 22 ++++++++++++++++++++++ > UefiCpuPkg/UefiCpuPkg.uni | 8 ++++++++ > 2 files changed, 30 insertions(+) > can you please write actual commit messages for your patches? In this series, neither patch has a commit message body. Minimally PcdCpuProcTraceMemSize and PcdCpuProcTraceOutputScheme should be named in both commit messages. I've noticed this as a tendency, and I feel it's now time for me to speak up. Consider the following earlier patch sets: * [edk2] [Patch v4 0/3] Enable Processor Trace feature https://lists.01.org/pipermail/edk2-devel/2017-August/012902.html No commit message body on either of the three patches. * [edk2] [Patch 0/3] Enable LMCE feature https://lists.01.org/pipermail/edk2-devel/2017-August/012731.html No commit message body on either of the three patches. Especially in the LMCE case, I tried to figure out how things fit together, but the commit messages gave zero information. IMO, without a commit message body, a patch cannot be considered complete (unless the patch is trivial). I feel that reviewers shouldn't accept empty commit messages either. Commit messages are not there (just) for the author, but for everyone else (too). Just because a patch is for UefiCpuPkg, someone else might stumble upon it when bisecting a regression, and they will want to read the justification. Commit messages also educate other project participants. It's not necessary to repeat the same old background in every single commit message, yes. But please provide pointers then, to specifications, text files, earlier commits (that do have detailed messages), and so on. Yes, writing commit messages takes time; sometimes even *more* time than writing the actual code. That's fine, it's part of open source development. Please invest the time. Thank you, Laszlo