Re: [Patch 0/2] Add comments for new Pcds

[Laszlo Ersek <lersek@redhat.com>]

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